[PATCH v2 0/6] MR5941: xcopy: Handle switch options concatenated with path.
-- v2: xcopy: Add support for parsing concatenated switches. xcopy: Handle switch options concatenated with path. xcopy: Introduce get_arg helper that duplicates first argument to new string. xcopy: Strip quotes only from source and destination arguments. xcopy: Exit on invalid command line argument. xcopy: Exit after displaying help message. https://gitlab.winehq.org/wine/wine/-/merge_requests/5941
From: Piotr Caban <piotr(a)codeweavers.com> --- programs/xcopy/xcopy.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/programs/xcopy/xcopy.c b/programs/xcopy/xcopy.c index 57adf1d6b84..202ce903407 100644 --- a/programs/xcopy/xcopy.c +++ b/programs/xcopy/xcopy.c @@ -703,22 +703,22 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, { DWORD flags = *pflags; WCHAR *cmdline, *word, *end, *next; - int rc = RC_INITERROR; + int rc; cmdline = _wcsdup(GetCommandLineW()); if (cmdline == NULL) - return rc; + exit(RC_INITERROR); /* Skip first arg, which is the program name */ if ((rc = find_end_of_word(cmdline, &word)) != RC_OK) - goto out; + exit(rc); word = skip_whitespace(word); while (*word) { WCHAR first; if ((rc = find_end_of_word(word, &end)) != RC_OK) - goto out; + exit(rc); next = skip_whitespace(end); first = word[0]; @@ -734,7 +734,7 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, lstrcpyW(supplieddestination, word); } else { XCOPY_wprintf(XCOPY_LoadMessage(STRING_INVPARMS)); - goto out; + exit(RC_INITERROR); } } else { /* Process all the switch options @@ -772,7 +772,7 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, &word[1], 8, L"EXCLUDE:", -1) == CSTR_EQUAL) { if (XCOPY_ProcessExcludeList(&word[9])) { XCOPY_FailMessage(ERROR_INVALID_PARAMETER); - goto out; + exit(RC_INITERROR); } else { flags |= OPT_EXCLUDELIST; @@ -831,7 +831,7 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, wine_dbgstr_w(datestring), wine_dbgstr_w(timestring)); } else { XCOPY_FailMessage(ERROR_INVALID_PARAMETER); - goto out; + exit(RC_INITERROR); } } else { flags |= OPT_DATENEWER; @@ -851,7 +851,7 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, default: WINE_TRACE("Unhandled parameter '%s'\n", wine_dbgstr_w(word)); XCOPY_wprintf(XCOPY_LoadMessage(STRING_INVPARM), word); - goto out; + exit(RC_INITERROR); } /* Unless overridden above, skip over the '/' and the first character */ @@ -876,11 +876,7 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, lstrcpyW(supplieddestination, L"."); *pflags = flags; - rc = RC_OK; - - out: - free(cmdline); - return rc; + return RC_OK; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5941
From: Piotr Caban <piotr(a)codeweavers.com> --- programs/xcopy/xcopy.c | 11 +++-------- programs/xcopy/xcopy.h | 1 - 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/programs/xcopy/xcopy.c b/programs/xcopy/xcopy.c index 1d003007c0b..57adf1d6b84 100644 --- a/programs/xcopy/xcopy.c +++ b/programs/xcopy/xcopy.c @@ -844,8 +844,7 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, } break; case '?': XCOPY_wprintf(XCOPY_LoadMessage(STRING_HELP)); - rc = RC_HELP; - goto out; + exit(RC_OK); case 'V': WINE_FIXME("ignoring /V\n"); break; @@ -1098,12 +1097,8 @@ int __cdecl wmain (int argc, WCHAR *argvW[]) * Parse the command line */ if ((rc = XCOPY_ParseCommandLine(suppliedsource, supplieddestination, - &flags)) != RC_OK) { - if (rc == RC_HELP) - return RC_OK; - else - return rc; - } + &flags)) != RC_OK) + return rc; /* Trace out the supplied information */ WINE_TRACE("Supplied parameters:\n"); diff --git a/programs/xcopy/xcopy.h b/programs/xcopy/xcopy.h index 9c6ee1176f5..1ed9e20c45b 100644 --- a/programs/xcopy/xcopy.h +++ b/programs/xcopy/xcopy.h @@ -26,7 +26,6 @@ #define RC_CTRLC 2 #define RC_INITERROR 4 #define RC_WRITEERROR 5 -#define RC_HELP 6 #define OPT_ASSUMEDIR 0x00000001 #define OPT_RECURSIVE 0x00000002 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5941
From: Piotr Caban <piotr(a)codeweavers.com> --- programs/xcopy/tests/xcopy.c | 5 +++++ programs/xcopy/xcopy.c | 19 ++++++++----------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/programs/xcopy/tests/xcopy.c b/programs/xcopy/tests/xcopy.c index fa3d5328a1b..2745ead97a5 100644 --- a/programs/xcopy/tests/xcopy.c +++ b/programs/xcopy/tests/xcopy.c @@ -106,6 +106,11 @@ static void test_parms_syntax(void) ok(GetFileAttributesA("xcopytest2") != INVALID_FILE_ATTRIBUTES, "xcopy failed to copy empty directory\n"); RemoveDirectoryA("xcopytest2"); + + rc = runcmd("xcopy xcopytest xcopytest2\\ /D/S/\"E\""); + ok(rc == 4, "xcopy /D/S/\"E\" test failed rc=%lu\n", rc); + ok(GetFileAttributesA("xcopytest2") == INVALID_FILE_ATTRIBUTES, + "xcopy copied empty directory incorrectly\n"); } static void test_keep_attributes(void) diff --git a/programs/xcopy/xcopy.c b/programs/xcopy/xcopy.c index 202ce903407..072d44728a3 100644 --- a/programs/xcopy/xcopy.c +++ b/programs/xcopy/xcopy.c @@ -684,18 +684,17 @@ static int find_end_of_word(const WCHAR *word, WCHAR **end) } /* Remove all double quotes from a word */ -static void strip_quotes(WCHAR *word, WCHAR **end) +static void strip_quotes(WCHAR *word) { - WCHAR *rp, *wp; - for (rp = word, wp = word; *rp != '\0'; rp++) { - if (*rp == '"') + WCHAR *wp; + for (wp = word; *word != '\0'; word++) { + if (*word == '"') continue; - if (wp < rp) - *wp = *rp; + if (wp < word) + *wp = *word; wp++; } *wp = '\0'; - *end = wp; } static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, @@ -716,18 +715,16 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, while (*word) { - WCHAR first; if ((rc = find_end_of_word(word, &end)) != RC_OK) exit(rc); next = skip_whitespace(end); - first = word[0]; *end = '\0'; - strip_quotes(word, &end); WINE_TRACE("Processing Arg: '%s'\n", wine_dbgstr_w(word)); /* First non-switch parameter is source, second is destination */ - if (first != '/') { + if (word[0] != '/') { + strip_quotes(word); if (suppliedsource[0] == 0x00) { lstrcpyW(suppliedsource, word); } else if (supplieddestination[0] == 0x00) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5941
From: Piotr Caban <piotr(a)codeweavers.com> --- programs/xcopy/xcopy.c | 67 +++++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 31 deletions(-) diff --git a/programs/xcopy/xcopy.c b/programs/xcopy/xcopy.c index 072d44728a3..715fd8f167b 100644 --- a/programs/xcopy/xcopy.c +++ b/programs/xcopy/xcopy.c @@ -647,7 +647,7 @@ static inline BOOL is_whitespace(WCHAR c) return c == ' ' || c == '\t'; } -static WCHAR *skip_whitespace(WCHAR *p) +static const WCHAR *skip_whitespace(const WCHAR *p) { for (; *p && is_whitespace(*p); p++); return p; @@ -662,10 +662,12 @@ static inline BOOL is_digit(WCHAR c) that lacks the escaped-quote logic of build_argv(), because literal double quotes are illegal in any of its arguments. Example: 'XCOPY "c:\DIR A" "c:DIR B\"' is OK. */ -static int find_end_of_word(const WCHAR *word, WCHAR **end) +static int get_arg(const WCHAR **cmdline, WCHAR **arg) { + const WCHAR *ptr = *cmdline; BOOL in_quotes = FALSE; - const WCHAR *ptr = word; + int len; + for (;;) { for (; *ptr != '\0' && *ptr != '"' && (in_quotes || !is_whitespace(*ptr)); ptr++); @@ -679,7 +681,15 @@ static int find_end_of_word(const WCHAR *word, WCHAR **end) if (*ptr == '\0' || (!in_quotes && is_whitespace(*ptr))) break; } - *end = (WCHAR*)ptr; + + len = ptr - *cmdline; + *arg = malloc((len + 1) * sizeof(WCHAR)); + if (!*arg) + return RC_INITERROR; + memcpy(*arg, *cmdline, len * sizeof(WCHAR)); + (*arg)[len] = 0; + + *cmdline = skip_whitespace(ptr); return RC_OK; } @@ -701,25 +711,20 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, WCHAR *supplieddestination, DWORD *pflags) { DWORD flags = *pflags; - WCHAR *cmdline, *word, *end, *next; + const WCHAR *cmdline; + WCHAR *word; int rc; - cmdline = _wcsdup(GetCommandLineW()); - if (cmdline == NULL) - exit(RC_INITERROR); - + cmdline = GetCommandLineW(); /* Skip first arg, which is the program name */ - if ((rc = find_end_of_word(cmdline, &word)) != RC_OK) + if ((rc = get_arg(&cmdline, &word)) != RC_OK) exit(rc); - word = skip_whitespace(word); + free(word); - while (*word) + while (*cmdline) { - if ((rc = find_end_of_word(word, &end)) != RC_OK) + if ((rc = get_arg(&cmdline, &word)) != RC_OK) exit(rc); - - next = skip_whitespace(end); - *end = '\0'; WINE_TRACE("Processing Arg: '%s'\n", wine_dbgstr_w(word)); /* First non-switch parameter is source, second is destination */ @@ -739,12 +744,12 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, but tests show it is done for each src file regardless of the destination */ int skip=0; - WCHAR *rest; + WCHAR *p = word, *rest; - while (word[0]) { + while (*p) { rest = NULL; - switch (toupper(word[1])) { + switch (toupper(p[1])) { case 'I': flags |= OPT_ASSUMEDIR; break; case 'S': flags |= OPT_RECURSIVE; break; case 'Q': flags |= OPT_QUIET; break; @@ -766,15 +771,15 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, /* E can be /E or /EXCLUDE */ case 'E': if (CompareStringW(LOCALE_USER_DEFAULT, NORM_IGNORECASE | SORT_STRINGSORT, - &word[1], 8, L"EXCLUDE:", -1) == CSTR_EQUAL) { - if (XCOPY_ProcessExcludeList(&word[9])) { + &p[1], 8, L"EXCLUDE:", -1) == CSTR_EQUAL) { + if (XCOPY_ProcessExcludeList(&p[9])) { XCOPY_FailMessage(ERROR_INVALID_PARAMETER); exit(RC_INITERROR); } else { flags |= OPT_EXCLUDELIST; /* Do not support concatenated switches onto exclude lists yet */ - rest = end; + rest = p + wcslen(p); } } else { flags |= OPT_EMPTYDIR | OPT_RECURSIVE; @@ -782,9 +787,9 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, break; /* D can be /D or /D: */ - case 'D': if (word[2]==':' && is_digit(word[3])) { + case 'D': if (p[2]==':' && is_digit(p[3])) { SYSTEMTIME st; - WCHAR *pos = &word[3]; + WCHAR *pos = &p[3]; BOOL isError = FALSE; memset(&st, 0x00, sizeof(st)); @@ -835,9 +840,9 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, } break; - case '-': if (toupper(word[2])=='Y') { + case '-': if (toupper(p[2])=='Y') { flags &= ~OPT_NOPROMPT; - rest = &word[3]; /* Skip over 3 characters */ + rest = &p[3]; /* Skip over 3 characters */ } break; case '?': XCOPY_wprintf(XCOPY_LoadMessage(STRING_HELP)); @@ -846,13 +851,13 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, WINE_FIXME("ignoring /V\n"); break; default: - WINE_TRACE("Unhandled parameter '%s'\n", wine_dbgstr_w(word)); - XCOPY_wprintf(XCOPY_LoadMessage(STRING_INVPARM), word); + WINE_TRACE("Unhandled parameter '%s'\n", wine_dbgstr_w(p)); + XCOPY_wprintf(XCOPY_LoadMessage(STRING_INVPARM), p); exit(RC_INITERROR); } /* Unless overridden above, skip over the '/' and the first character */ - if (rest == NULL) rest = &word[2]; + if (rest == NULL) rest = &p[2]; /* By now, rest should point either to the null after the switch, or the beginning of the next switch if there @@ -861,11 +866,11 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, WINE_FIXME("Unexpected characters found and ignored '%s'\n", wine_dbgstr_w(rest)); skip=1; } else { - word = rest; + p = rest; } } } - word = next; + free(word); } /* Default the destination if not supplied */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5941
From: Piotr Caban <piotr(a)codeweavers.com> --- programs/xcopy/tests/xcopy.c | 2 +- programs/xcopy/xcopy.c | 17 ++++++----------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/programs/xcopy/tests/xcopy.c b/programs/xcopy/tests/xcopy.c index 2745ead97a5..4c51f941dfc 100644 --- a/programs/xcopy/tests/xcopy.c +++ b/programs/xcopy/tests/xcopy.c @@ -101,7 +101,7 @@ static void test_parms_syntax(void) ok(GetFileAttributesA("xcopytest2") == INVALID_FILE_ATTRIBUTES, "xcopy copied empty directory incorrectly\n"); - rc = runcmd("xcopy /D/S/E xcopytest xcopytest2\\"); + rc = runcmd("xcopy xcopytest xcopytest2\\/D/S/E"); ok(rc == 0, "xcopy /D/S/E test failed rc=%lu\n", rc); ok(GetFileAttributesA("xcopytest2") != INVALID_FILE_ATTRIBUTES, "xcopy failed to copy empty directory\n"); diff --git a/programs/xcopy/xcopy.c b/programs/xcopy/xcopy.c index 715fd8f167b..84f2d312b6f 100644 --- a/programs/xcopy/xcopy.c +++ b/programs/xcopy/xcopy.c @@ -665,21 +665,16 @@ static inline BOOL is_digit(WCHAR c) static int get_arg(const WCHAR **cmdline, WCHAR **arg) { const WCHAR *ptr = *cmdline; - BOOL in_quotes = FALSE; int len; - for (;;) { - for (; *ptr != '\0' && *ptr != '"' && - (in_quotes || !is_whitespace(*ptr)); ptr++); + if (*ptr == '/') ptr++; + while (*ptr && !is_whitespace(*ptr) && *ptr != '/') { if (*ptr == '"') { - in_quotes = !in_quotes; - ptr++; + while (*ptr && *ptr != '"') ptr++; + /* Odd number of double quotes is illegal for XCOPY */ + if (!*ptr) return RC_INITERROR; } - /* Odd number of double quotes is illegal for XCOPY */ - if (in_quotes && *ptr == '\0') - return RC_INITERROR; - if (*ptr == '\0' || (!in_quotes && is_whitespace(*ptr))) - break; + ptr++; } len = ptr - *cmdline; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5941
From: Piotr Caban <piotr(a)codeweavers.com> --- programs/xcopy/tests/xcopy.c | 6 ++++++ programs/xcopy/xcopy.c | 34 ++++++++++------------------------ 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/programs/xcopy/tests/xcopy.c b/programs/xcopy/tests/xcopy.c index 4c51f941dfc..61fbad9ccaf 100644 --- a/programs/xcopy/tests/xcopy.c +++ b/programs/xcopy/tests/xcopy.c @@ -111,6 +111,12 @@ static void test_parms_syntax(void) ok(rc == 4, "xcopy /D/S/\"E\" test failed rc=%lu\n", rc); ok(GetFileAttributesA("xcopytest2") == INVALID_FILE_ATTRIBUTES, "xcopy copied empty directory incorrectly\n"); + + rc = runcmd("xcopy xcopytest xcopytest2\\/DSE"); + ok(rc == 0, "xcopy /DSE test failed rc=%lu\n", rc); + ok(GetFileAttributesA("xcopytest2") != INVALID_FILE_ATTRIBUTES, + "xcopy failed to copy empty directory\n"); + RemoveDirectoryA("xcopytest2"); } static void test_keep_attributes(void) diff --git a/programs/xcopy/xcopy.c b/programs/xcopy/xcopy.c index 84f2d312b6f..46a2727dd49 100644 --- a/programs/xcopy/xcopy.c +++ b/programs/xcopy/xcopy.c @@ -738,13 +738,12 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, Note: Windows docs say /P prompts when dest is created but tests show it is done for each src file regardless of the destination */ - int skip=0; - WCHAR *p = word, *rest; + WCHAR *p = word + 1, *rest; while (*p) { - rest = NULL; + rest = p + 1; - switch (toupper(p[1])) { + switch (toupper(*p)) { case 'I': flags |= OPT_ASSUMEDIR; break; case 'S': flags |= OPT_RECURSIVE; break; case 'Q': flags |= OPT_QUIET; break; @@ -766,14 +765,12 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, /* E can be /E or /EXCLUDE */ case 'E': if (CompareStringW(LOCALE_USER_DEFAULT, NORM_IGNORECASE | SORT_STRINGSORT, - &p[1], 8, L"EXCLUDE:", -1) == CSTR_EQUAL) { - if (XCOPY_ProcessExcludeList(&p[9])) { + p, 8, L"EXCLUDE:", -1) == CSTR_EQUAL) { + if (XCOPY_ProcessExcludeList(&p[8])) { XCOPY_FailMessage(ERROR_INVALID_PARAMETER); exit(RC_INITERROR); } else { flags |= OPT_EXCLUDELIST; - - /* Do not support concatenated switches onto exclude lists yet */ rest = p + wcslen(p); } } else { @@ -782,9 +779,9 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, break; /* D can be /D or /D: */ - case 'D': if (p[2]==':' && is_digit(p[3])) { + case 'D': if (p[1]==':' && is_digit(p[2])) { SYSTEMTIME st; - WCHAR *pos = &p[3]; + WCHAR *pos = &p[2]; BOOL isError = FALSE; memset(&st, 0x00, sizeof(st)); @@ -835,9 +832,9 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, } break; - case '-': if (toupper(p[2])=='Y') { + case '-': if (toupper(p[1])=='Y') { flags &= ~OPT_NOPROMPT; - rest = &p[3]; /* Skip over 3 characters */ + rest = &p[2]; /* Skip over 2 characters */ } break; case '?': XCOPY_wprintf(XCOPY_LoadMessage(STRING_HELP)); @@ -851,18 +848,7 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, exit(RC_INITERROR); } - /* Unless overridden above, skip over the '/' and the first character */ - if (rest == NULL) rest = &p[2]; - - /* By now, rest should point either to the null after the - switch, or the beginning of the next switch if there - was no whitespace between them */ - if (!skip && *rest && *rest != '/') { - WINE_FIXME("Unexpected characters found and ignored '%s'\n", wine_dbgstr_w(rest)); - skip=1; - } else { - p = rest; - } + p = rest; } } free(word); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5941
I have added more improvements to xcopy command line parsing. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5941#note_75015
participants (2)
-
Piotr Caban -
Piotr Caban (＠piotr)