Signed-off-by: Mohamad Al-Jaf mohamadaljaf@gmail.com --- This is a lot more readable and easily allows for additional expressions to be supported if needed in the future. It also conforms to the general code style in wusa. --- programs/wusa/main.c | 60 ++++++++++++++++++++++++-------------------- programs/wusa/wusa.h | 8 ++++++ 2 files changed, 41 insertions(+), 27 deletions(-)
diff --git a/programs/wusa/main.c b/programs/wusa/main.c index 74dfeb68d5f..f43e86c0019 100644 --- a/programs/wusa/main.c +++ b/programs/wusa/main.c @@ -52,6 +52,31 @@ struct installer_state struct list updates; };
+static const struct expression expression_table[] = +{ + {L"runtime.system32", L"amd64", CSIDL_SYSTEM, NULL}, + {L"runtime.system32", L"x86", CSIDL_SYSTEMX86, NULL}, + {L"runtime.system32", L"wow64", CSIDL_SYSTEMX86, NULL}, + {L"runtime.windows", NULL, CSIDL_WINDOWS, NULL}, + {L"runtime.programfiles", L"amd64", CSIDL_PROGRAM_FILES, NULL}, + {L"runtime.programfiles", L"x86", CSIDL_PROGRAM_FILESX86, NULL}, + {L"runtime.programfiles", L"wow64", CSIDL_PROGRAM_FILESX86, NULL}, + {L"runtime.programfilesX86", L"x86", CSIDL_PROGRAM_FILESX86, NULL}, + {L"runtime.commonfiles", L"amd64", CSIDL_PROGRAM_FILES_COMMON, NULL}, + {L"runtime.commonfiles", L"x86", CSIDL_PROGRAM_FILES_COMMONX86, NULL}, + {L"runtime.commonfiles", L"wow64", CSIDL_PROGRAM_FILES_COMMONX86, NULL}, + {L"runtime.commonfilesX86", L"x86", CSIDL_PROGRAM_FILES_COMMONX86, NULL}, + {L"runtime.programdata", NULL, CSIDL_COMMON_APPDATA, NULL}, + {L"runtime.drivers", L"amd64", CSIDL_SYSTEM, L"\drivers"}, + {L"runtime.drivers", L"x86", CSIDL_SYSTEMX86, L"\drivers"}, + {L"runtime.drivers", L"wow64", CSIDL_SYSTEMX86, L"\drivers"}, + {L"runtime.wbem", L"amd64", CSIDL_SYSTEM, L"\wbem"}, + {L"runtime.wbem", L"x86", CSIDL_SYSTEMX86, L"\wbem"}, + {L"runtime.wbem", L"wow64", CSIDL_SYSTEMX86, L"\wbem"}, + {L"runtime.inf", NULL, CSIDL_WINDOWS, L"\inf"}, + {L"runtime.fonts", NULL, CSIDL_FONTS, NULL}, +}; + static void * CDECL cabinet_alloc(ULONG cb) { return malloc(cb); @@ -480,36 +505,19 @@ static BOOL strbuf_append(struct strbuf *buf, const WCHAR *str, DWORD len) static WCHAR *lookup_expression(struct assembly_entry *assembly, const WCHAR *key) { WCHAR path[MAX_PATH]; + unsigned int i; int csidl = 0;
- if (!wcsicmp(key, L"runtime.system32") || !wcsicmp(key, L"runtime.drivers") || !wcsicmp(key, L"runtime.wbem")) - { -#ifdef __x86_64__ - if (!wcsicmp(assembly->identity.architecture, L"x86")) csidl = CSIDL_SYSTEMX86; -#endif - if (!csidl) csidl = CSIDL_SYSTEM; - } - else if (!wcsicmp(key, L"runtime.windows") || !wcsicmp(key, L"runtime.inf")) csidl = CSIDL_WINDOWS; - else if (!wcsicmp(key, L"runtime.programfiles")) + for (i = 0; i < ARRAY_SIZE(expression_table); ++i) { + if (wcsicmp(key, expression_table[i].key)) continue; #ifdef __x86_64__ - if (!wcsicmp(assembly->identity.architecture, L"x86")) csidl = CSIDL_PROGRAM_FILESX86; + if (expression_table[i].architecture) + if (wcsicmp(assembly->identity.architecture, expression_table[i].architecture)) continue; #endif - if (!csidl) csidl = CSIDL_PROGRAM_FILES; - } - else if (!wcsicmp(key, L"runtime.commonfiles")) - { -#ifdef __x86_64__ - if (!wcsicmp(assembly->identity.architecture, L"x86")) csidl = CSIDL_PROGRAM_FILES_COMMONX86; -#endif - if (!csidl) csidl = CSIDL_PROGRAM_FILES_COMMON; + csidl = expression_table[i].csidl; + break; } -#ifdef __x86_64__ - else if (!wcsicmp(key, L"runtime.programfilesx86")) csidl = CSIDL_PROGRAM_FILESX86; - else if (!wcsicmp(key, L"runtime.commonfilesx86")) csidl = CSIDL_PROGRAM_FILES_COMMONX86; -#endif - else if (!wcsicmp(key, L"runtime.programdata")) csidl = CSIDL_COMMON_APPDATA; - else if (!wcsicmp(key, L"runtime.fonts")) csidl = CSIDL_FONTS;
if (!csidl) { @@ -522,9 +530,7 @@ static WCHAR *lookup_expression(struct assembly_entry *assembly, const WCHAR *ke return NULL; }
- if (!wcsicmp(key, L"runtime.inf")) wcscat(path, L"\inf"); - else if (!wcsicmp(key, L"runtime.drivers")) wcscat(path, L"\drivers"); - else if (!wcsicmp(key, L"runtime.wbem")) wcscat(path, L"\wbem"); + if (expression_table[i].append) wcscat(path, expression_table[i].append); return strdupW(path); }
diff --git a/programs/wusa/wusa.h b/programs/wusa/wusa.h index da679a27f07..fa8e37979e2 100644 --- a/programs/wusa/wusa.h +++ b/programs/wusa/wusa.h @@ -33,6 +33,14 @@ struct assembly_identity WCHAR *pubkey_token; };
+struct expression +{ + const WCHAR *key; + const WCHAR *architecture; + int csidl; + const WCHAR *append; +}; + struct dependency_entry { struct list entry;
On Sun, 2022-04-03 at 22:12 -0400, Mohamad Al-Jaf wrote:
Signed-off-by: Mohamad Al-Jaf mohamadaljaf@gmail.com
This is a lot more readable and easily allows for additional expressions to be supported if needed in the future. It also conforms to the general code style in wusa.
I'm not sure this is worth it. I don't expect many more entries to be added to this list and from a certain perpective it's less clear than before because you need to look in two places to understand when a path is overridden.
On Tue, Apr 5, 2022, 07:37 Hans Leidekker hans@codeweavers.com wrote:
I'm not sure this is worth it. I don't expect many more entries to be added to this list and from a certain perpective it's less clear than before because you need to look in two places to understand when a path is overridden.
I agree that more entries probably won't be added but doesn't the current implementation return the wrong path for wow64 architecture? Perhaps you've already accounted for that and I missed it.
Whatever the case I'll respect your decision.
Also, sorry if the formatting in this email is off, I'm replying from my phone.
On Tue, 2022-04-05 at 11:11 -0400, Mohamad Al-Jaf wrote:
On Tue, Apr 5, 2022, 07:37 Hans Leidekker hans@codeweavers.com wrote:
I'm not sure this is worth it. I don't expect many more entries to be added to this list and from a certain perpective it's less clear than before because you need to look in two places to understand when a path is overridden.
I agree that more entries probably won't be added but doesn't the current implementation return the wrong path for wow64 architecture? Perhaps you've already accounted for that and I missed it
I saw you added 'wow64' as a synonym for 'x86'. Do you have an installer that needs this? In any case, this should go in a separate patch.
On Tue, Apr 5, 2022 at 11:26 AM Hans Leidekker hans@codeweavers.com wrote:
I saw you added 'wow64' as a synonym for 'x86'. Do you have an installer that needs this? In any case, this should go in a separate patch.
Yeah, it's the same update package that had the unsupported expressions, Microsoft-Windows-MediaFeaturePack-OOB-Package_x64.msu. I can submit a separate patch for the wow64 architecture, but I'm not sure how to support it. Do I just add an extra or case in the existing if statements or is there a better way? Maybe assigning the assembly identity architecture to x86 if it's wow64, but this feels dirty. This expression table would solve that issue while keeping the code clean. What do you think? -- Kind regards, Mohamad
On Tue, 2022-04-05 at 22:49 -0400, Mohamad Al-Jaf wrote:
On Tue, Apr 5, 2022 at 11:26 AM Hans Leidekker hans@codeweavers.com wrote:
I saw you added 'wow64' as a synonym for 'x86'. Do you have an installer that needs this? In any case, this should go in a separate patch.
Yeah, it's the same update package that had the unsupported expressions, Microsoft-Windows-MediaFeaturePack-OOB-Package_x64.msu. I can submit a separate patch for the wow64 architecture, but I'm not sure how to support it. Do I just add an extra or case in the existing if statements or is there a better way? Maybe assigning the assembly identity architecture to x86 if it's wow64, but this feels dirty. This expression table would solve that issue while keeping the code clean. What do you think?
You could add an is_x86() helper that returns true if the assembly architecture matches "x86" or "wow64".
On Wed, Apr 6, 2022 at 3:59 AM Hans Leidekker hans@codeweavers.com wrote:
You could add an is_x86() helper that returns true if the assembly architecture matches "x86" or "wow64".
This causes warnings when compiling the 32-bit build:
warning: ‘expression_is_x86’ defined but not used [-Wunused-function] 505 | static BOOL expression_is_x86(const WCHAR *str)
Is this fine? -- Kind regards, Mohamad
On Wed, 2022-04-06 at 04:44 -0400, Mohamad Al-Jaf wrote:
On Wed, Apr 6, 2022 at 3:59 AM Hans Leidekker hans@codeweavers.com wrote:
You could add an is_x86() helper that returns true if the assembly architecture matches "x86" or "wow64".
This causes warnings when compiling the 32-bit build:
warning: ‘expression_is_x86’ defined but not used [-Wunused-function] 505 | static BOOL expression_is_x86(const WCHAR *str)
Is this fine?
No, there should be no warnings.
-
Hans Leidekker -
Mohamad Al-Jaf