On Tuesday, 24 May 2016 11:35 PM, Alexandre Julliard wrote:
Hugh McMaster writes:
@@ -907,19 +915,16 @@ int wmain(int argc, WCHAR *argvW[]) return 0; }
- option_help = (!lstrcmpW(argvW[2], slashHelpW) || !lstrcmpiW(argvW[2], slashHW));
You should check argc first.
Ah, yes, I missed that.
if (argc < 3){output_message(STRING_INVALID_SYNTAX);output_message(STRING_FUNC_HELP, struprW(argvW[1]));return 1;}else if (argc == 3 && (!lstrcmpW(argvW[2], slashHelpW) ||!lstrcmpiW(argvW[2], slashHW)))
if (argc < 3 || (argc > 3 && option_help))return invalid_syntax(argvW[1]);else if (option_help) {This doesn't feel like much of an improvement. You could probably come up with more useful helper functions.
Ideally, I'd like to move all of that logic out of the 'if' block string comparisons. One major issue I see is that users may pass invalid operation names, e.g. abc, so we need to ensure that we only continue with valid strings.
I believe this will also cause problems when functions like sane_path() are moved into wmain() as well, so it's better to deal with this now.
One solution is to iterate over an array of WCHAR * and return a BOOL indicating whether the string is valid or not.
As for passing output_message() the correct help string ID, leaving that code where it is now is probably best, particularly if I can use the BOOL option_help. If not, we'd have to look up the message ID information in a function or a struct. I'm not if you think this change would have any benefit.