Fixes bug 45052
Add support for 'if exist filenam*' style statments with associated tests
Signed-off-by: Jason Edmeades us@edmeades.me.uk --- programs/cmd/builtins.c | 8 +++-- programs/cmd/tests/test_builtins.cmd | 43 ++++++++++++++++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 8 +++++ 3 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 04b098e98d..541915df67 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -2804,8 +2804,12 @@ void WCMD_if (WCHAR *p, CMD_LIST **cmdList) WCMD_parameter(p, 2+negate, &command, FALSE, FALSE); } else if (!lstrcmpiW (condition, existW)) { - test = (GetFileAttributesW(WCMD_parameter(p, 1+negate, NULL, FALSE, FALSE)) - != INVALID_FILE_ATTRIBUTES); + HANDLE hff = INVALID_HANDLE_VALUE; + WIN32_FIND_DATAW fd; + hff = FindFirstFileW(WCMD_parameter(p, 1+negate, NULL, FALSE, FALSE), &fd); + test = (hff != INVALID_HANDLE_VALUE ); + if (!test) FindClose(hff); + WCMD_parameter(p, 2+negate, &command, FALSE, FALSE); } else if (!lstrcmpiW (condition, defdW)) { diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 63ec3cacb1..d6edc5fff1 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -979,6 +979,49 @@ for %%i in (%WINE_STR_PARMS%) do ( for %%i in (%WINE_STR_PARMS%) do ( for %%j in (%WINE_STR_PARMS%) do ( call :GTRtest %%i %%j)) + +echo ------------ Testing if/exist ------------ +mkdir subdir +echo something>subdir\bar +echo something else>foo +if exist foo ( + echo exist explicit works +) else ( + echo ERROR exist explicit broken +) +if exist bar ( + echo ERROR exist explicit unknown file broken +) else ( + echo exist explicit unknown file works +) +if exist subdir\bar ( + echo exist explicit in subdir works +) else ( + echo ERROR exist explicit in subdir broken +) +if exist fo* ( + echo exist simple wildcard works +) else ( + echo ERROR exist simple wildcard broken +) +if exist subdir\ba* ( + echo exist wildcard works +) else ( + echo ERROR exist wildcard broken +) +if not exist subdir\ba* ( + echo ERROR negate exist wildcard broken +) else ( + echo negate exist wildcard works +) +if exist idontexist\ba* ( + echo ERROR exist wildcard bad subdir broken +) else ( + echo exist wildcard bad subdir broken works +) +del foo subdir\bar +rd subdir + echo ------ for numbers if -1 LSS 1 (echo negative numbers handled) if not -1 LSS -10 (echo negative numbers handled) diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index dcc96299b9..22d83c3b19 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -768,6 +768,14 @@ BA GTR B BA GTR AB BA GTR AA AA GTR A +------------ Testing if/exist ------------ +exist explicit works +exist explicit unknown file works +exist explicit in subdir works +exist simple wildcard works +exist wildcard works +negate exist wildcard works +exist wildcard bad subdir broken works ------ for numbers negative numbers handled negative numbers handled
Fixes bug 45051
A for loop can be working through a wildcarded subdirectory, but when processing the first file in the subdirectory, it stores the prefix in a static variable which gets overwritten during the 'for' body processing. Take a copy on the stack to free up the static buffer
Signed-off-by: Jason Edmeades us@edmeades.me.uk --- programs/cmd/builtins.c | 10 ++++++++-- programs/cmd/tests/test_builtins.cmd | 7 +++++++ programs/cmd/tests/test_builtins.cmd.exp | 4 ++++ 3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 541915df67..56c50790ca 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -2261,19 +2261,25 @@ void WCMD_for (WCHAR *p, CMD_LIST **cmdList) { thisSet->bracketDepth >= thisDepth) {
/* Loop through all entries on the same line */ - WCHAR *item; + WCHAR *staticitem; WCHAR *itemStart; WCHAR buffer[MAXSTRING];
WINE_TRACE("Processing for set %p\n", thisSet); i = 0; - while (*(item = WCMD_parameter (thisSet->command, i, &itemStart, TRUE, FALSE))) { + while (*(staticitem = WCMD_parameter (thisSet->command, i, &itemStart, TRUE, FALSE))) {
/* * If the parameter within the set has a wildcard then search for matching files * otherwise do a literal substitution. */ static const WCHAR wildcards[] = {'*','?','\0'}; + + /* Take a copy of the item returned from WCMD_parameter as it is held in a + static buffer which can be overwritten during parsing of the for body */ + WCHAR item[MAXSTRING]; + strcpyW(item, staticitem); + thisCmdStart = cmdStart;
itemNum++; diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index d6edc5fff1..6f2ef4a843 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1155,9 +1155,16 @@ mkdir foobar & cd foobar mkdir foo mkdir bar mkdir baz +mkdir pop echo > bazbaz echo --- basic wildcards for %%i in (ba*) do echo %%i +echo --- wildcards in subdirs +echo something>pop\bar1 +echo something>pop\bar2.txt +echo something>pop\bar3 +for %%f in (pop\ba*) do ( call echo %%f ) +rmdir /s/q pop echo --- for /d for /d %%i in (baz foo bar) do echo %%i 2>&1 rem Confirm we don't match files: diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 22d83c3b19..0eb5b966e8 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -917,6 +917,10 @@ B C B D --- basic wildcards bazbaz +--- wildcards in subdirs +pop\bar1@space@ +pop\bar2.txt@space@ +pop\bar3@space@ --- for /d baz@space@ foo@space@
Fixes bug#44967
This changes xcopy to support flags supplied like /S/E without spaces between them. Tests supplied but in writing the tests I found a few other problems left as todo in this patch.
(Note the majority of this patch is just changing the indenting of a large code block)
Signed-off-by: Jason Edmeades us@edmeades.me.uk --- programs/xcopy/tests/xcopy.c | 39 +++++++ programs/xcopy/xcopy.c | 208 ++++++++++++++++++++--------------- 2 files changed, 159 insertions(+), 88 deletions(-)
diff --git a/programs/xcopy/tests/xcopy.c b/programs/xcopy/tests/xcopy.c index ec7683aecc..7ca32d6c33 100644 --- a/programs/xcopy/tests/xcopy.c +++ b/programs/xcopy/tests/xcopy.c @@ -73,6 +73,44 @@ static void test_date_format(void) DeleteFileA("xcopytest\xcopy1"); }
+static void test_parms_syntax(void) +{ + DWORD rc; + + rc = runcmd("xcopy /H/D:20-01-2000 xcopy1 xcopytest"); + ok(rc == 4, "xcopy /H/D:d-m-y test returned rc=%u\n", rc); + ok(GetFileAttributesA("xcopytest\xcopy1") == INVALID_FILE_ATTRIBUTES, + "xcopy should not have created xcopytest\xcopy1\n"); + + rc = runcmd("xcopy /D:01-20-2000/H xcopy1 xcopytest"); + ok(rc == 0, "xcopy /H/D:m-d-y test failed rc=%u\n", rc); + ok(GetFileAttributesA("xcopytest\xcopy1") != INVALID_FILE_ATTRIBUTES, + "xcopy did not create xcopytest\xcopy1\n"); + DeleteFileA("xcopytest\xcopy1"); + + /* The following test is commented out as under wine it generates + a recursively deep directory tree (todo_wine) + rc = runcmd("xcopy /D:1-20-2000/E xcopy1 xcopytest"); + ok(rc == 0, "xcopy /D:m-d-y/E test failed rc=%u\n", rc); + ok(GetFileAttributesA("xcopytest\xcopy1") != INVALID_FILE_ATTRIBUTES, + "xcopy did not create xcopytest\xcopy1\n"); + DeleteFileA("xcopytest\xcopy1"); */ + + rc = runcmd("xcopy /D/S xcopytest xcopytest2\"); + todo_wine + ok(rc == 0, "xcopy /D/S test failed rc=%u\n", rc); + ok(GetFileAttributesA("xcopytest2") == INVALID_FILE_ATTRIBUTES, + "xcopy copied empty directory incorrectly\n"); + + rc = runcmd("xcopy /D/S/E xcopytest xcopytest2\"); + todo_wine { + ok(rc == 0, "xcopy /D/S/E test failed rc=%u\n", rc); + ok(GetFileAttributesA("xcopytest2") != INVALID_FILE_ATTRIBUTES, + "xcopy failed to copy empty directory\n"); + } + RemoveDirectoryA("xcopytest2"); +} + START_TEST(xcopy) { char tmpdir[MAX_PATH]; @@ -94,6 +132,7 @@ START_TEST(xcopy) CloseHandle(hfile);
test_date_format(); + test_parms_syntax();
DeleteFileA("xcopy1"); RemoveDirectoryA("xcopytest"); diff --git a/programs/xcopy/xcopy.c b/programs/xcopy/xcopy.c index a173cc14c7..461104a8da 100644 --- a/programs/xcopy/xcopy.c +++ b/programs/xcopy/xcopy.c @@ -743,101 +743,133 @@ 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 */ - switch (toupper(word[1])) { - case 'I': flags |= OPT_ASSUMEDIR; break; - case 'S': flags |= OPT_RECURSIVE; break; - case 'Q': flags |= OPT_QUIET; break; - case 'F': flags |= OPT_FULL; break; - case 'L': flags |= OPT_SIMULATE; break; - case 'W': flags |= OPT_PAUSE; break; - case 'T': flags |= OPT_NOCOPY | OPT_RECURSIVE; break; - case 'Y': flags |= OPT_NOPROMPT; break; - case 'N': flags |= OPT_SHORTNAME; break; - case 'U': flags |= OPT_MUSTEXIST; break; - case 'R': flags |= OPT_REPLACEREAD; break; - case 'H': flags |= OPT_COPYHIDSYS; break; - case 'C': flags |= OPT_IGNOREERRORS; break; - case 'P': flags |= OPT_SRCPROMPT; break; - case 'A': flags |= OPT_ARCHIVEONLY; break; - case 'M': flags |= OPT_ARCHIVEONLY | - OPT_REMOVEARCH; break; - - /* E can be /E or /EXCLUDE */ - case 'E': if (CompareStringW(LOCALE_USER_DEFAULT, - NORM_IGNORECASE | SORT_STRINGSORT, - &word[1], 8, - EXCLUDE, -1) == CSTR_EQUAL) { - if (XCOPY_ProcessExcludeList(&word[9])) { - XCOPY_FailMessage(ERROR_INVALID_PARAMETER); - goto out; - } else flags |= OPT_EXCLUDELIST; - } else flags |= OPT_EMPTYDIR | OPT_RECURSIVE; - break; - - /* D can be /D or /D: */ - case 'D': if (word[2]==':' && is_digit(word[3])) { - SYSTEMTIME st; - WCHAR *pos = &word[3]; - BOOL isError = FALSE; - memset(&st, 0x00, sizeof(st)); - - /* Microsoft xcopy's usage message implies that the date - * format depends on the locale, but that is false. - * It is hardcoded to month-day-year. - */ - st.wMonth = _wtol(pos); - while (*pos && is_digit(*pos)) pos++; - if (*pos++ != '-') isError = TRUE; - - if (!isError) { - st.wDay = _wtol(pos); - while (*pos && is_digit(*pos)) pos++; - if (*pos++ != '-') isError = TRUE; - } + int skip=0; + WCHAR *rest; + + while (word[0]) { + rest = NULL; + + switch (toupper(word[1])) { + case 'I': flags |= OPT_ASSUMEDIR; break; + case 'S': flags |= OPT_RECURSIVE; break; + case 'Q': flags |= OPT_QUIET; break; + case 'F': flags |= OPT_FULL; break; + case 'L': flags |= OPT_SIMULATE; break; + case 'W': flags |= OPT_PAUSE; break; + case 'T': flags |= OPT_NOCOPY | OPT_RECURSIVE; break; + case 'Y': flags |= OPT_NOPROMPT; break; + case 'N': flags |= OPT_SHORTNAME; break; + case 'U': flags |= OPT_MUSTEXIST; break; + case 'R': flags |= OPT_REPLACEREAD; break; + case 'H': flags |= OPT_COPYHIDSYS; break; + case 'C': flags |= OPT_IGNOREERRORS; break; + case 'P': flags |= OPT_SRCPROMPT; break; + case 'A': flags |= OPT_ARCHIVEONLY; break; + case 'M': flags |= OPT_ARCHIVEONLY | + OPT_REMOVEARCH; break; + + /* E can be /E or /EXCLUDE */ + case 'E': if (CompareStringW(LOCALE_USER_DEFAULT, + NORM_IGNORECASE | SORT_STRINGSORT, + &word[1], 8, + EXCLUDE, -1) == CSTR_EQUAL) { + if (XCOPY_ProcessExcludeList(&word[9])) { + XCOPY_FailMessage(ERROR_INVALID_PARAMETER); + goto out; + } else { + flags |= OPT_EXCLUDELIST;
- if (!isError) { - st.wYear = _wtol(pos); - while (*pos && is_digit(*pos)) pos++; - if (st.wYear < 100) st.wYear+=2000; + /* Do not support concatenated switches onto exclude lists yet */ + rest = end; + } + } else { + flags |= OPT_EMPTYDIR | OPT_RECURSIVE; } + break;
- if (!isError && SystemTimeToFileTime(&st, &dateRange)) { + /* D can be /D or /D: */ + case 'D': if (word[2]==':' && is_digit(word[3])) { SYSTEMTIME st; - WCHAR datestring[32], timestring[32]; - - flags |= OPT_DATERANGE; - - /* Debug info: */ - FileTimeToSystemTime (&dateRange, &st); - GetDateFormatW(0, DATE_SHORTDATE, &st, NULL, datestring, - sizeof(datestring)/sizeof(WCHAR)); - GetTimeFormatW(0, TIME_NOSECONDS, &st, - NULL, timestring, sizeof(timestring)/sizeof(WCHAR)); + WCHAR *pos = &word[3]; + BOOL isError = FALSE; + memset(&st, 0x00, sizeof(st)); + + /* Microsoft xcopy's usage message implies that the date + * format depends on the locale, but that is false. + * It is hardcoded to month-day-year. + */ + st.wMonth = _wtol(pos); + while (*pos && is_digit(*pos)) pos++; + if (*pos++ != '-') isError = TRUE;
- WINE_TRACE("Date being used is: %s %s\n", - wine_dbgstr_w(datestring), wine_dbgstr_w(timestring)); + if (!isError) { + st.wDay = _wtol(pos); + while (*pos && is_digit(*pos)) pos++; + if (*pos++ != '-') isError = TRUE; + } + + if (!isError) { + st.wYear = _wtol(pos); + while (*pos && is_digit(*pos)) pos++; + if (st.wYear < 100) st.wYear+=2000; + } + + /* Handle switches straight after the supplied date */ + rest = pos; + + if (!isError && SystemTimeToFileTime(&st, &dateRange)) { + SYSTEMTIME st; + WCHAR datestring[32], timestring[32]; + + flags |= OPT_DATERANGE; + + /* Debug info: */ + FileTimeToSystemTime (&dateRange, &st); + GetDateFormatW(0, DATE_SHORTDATE, &st, NULL, datestring, + sizeof(datestring)/sizeof(WCHAR)); + GetTimeFormatW(0, TIME_NOSECONDS, &st, + NULL, timestring, sizeof(timestring)/sizeof(WCHAR)); + + WINE_TRACE("Date being used is: %s %s\n", + wine_dbgstr_w(datestring), wine_dbgstr_w(timestring)); + } else { + XCOPY_FailMessage(ERROR_INVALID_PARAMETER); + goto out; + } } else { - XCOPY_FailMessage(ERROR_INVALID_PARAMETER); - goto out; + flags |= OPT_DATENEWER; } - } else { - flags |= OPT_DATENEWER; - } - break; - - case '-': if (toupper(word[2])=='Y') - flags &= ~OPT_NOPROMPT; - break; - case '?': XCOPY_wprintf(XCOPY_LoadMessage(STRING_HELP)); - rc = RC_HELP; - goto out; - case 'V': - WINE_FIXME("ignoring /V\n"); - break; - default: - WINE_TRACE("Unhandled parameter '%s'\n", wine_dbgstr_w(word)); - XCOPY_wprintf(XCOPY_LoadMessage(STRING_INVPARM), word); - goto out; + break; + + case '-': if (toupper(word[2])=='Y') { + flags &= ~OPT_NOPROMPT; + rest = &word[3]; /* Skip over 3 characters */ + } + break; + case '?': XCOPY_wprintf(XCOPY_LoadMessage(STRING_HELP)); + rc = RC_HELP; + goto out; + case 'V': + WINE_FIXME("ignoring /V\n"); + break; + default: + WINE_TRACE("Unhandled parameter '%s'\n", wine_dbgstr_w(word)); + XCOPY_wprintf(XCOPY_LoadMessage(STRING_INVPARM), word); + goto out; + } + + /* Unless overriden above, skip over the '/' and the first character */ + if (rest == NULL) rest = &word[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 { + word = rest; + } } } word = next;
Found whilst fixing bug#44967
Testing, and confirmed on the internet (*1) shows xcopy never returns 1 to indicate no files were copied.
(*1) e.g. https://superuser.com/questions/1180180/is-it-possible-to-get-an-errorlevel-...
Signed-off-by: Jason Edmeades us@edmeades.me.uk --- programs/xcopy/tests/xcopy.c | 4 +--- programs/xcopy/xcopy.c | 7 ++++--- 2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/programs/xcopy/tests/xcopy.c b/programs/xcopy/tests/xcopy.c index 7ca32d6c33..0991d49769 100644 --- a/programs/xcopy/tests/xcopy.c +++ b/programs/xcopy/tests/xcopy.c @@ -97,17 +97,15 @@ static void test_parms_syntax(void) DeleteFileA("xcopytest\xcopy1"); */
rc = runcmd("xcopy /D/S xcopytest xcopytest2\"); - todo_wine ok(rc == 0, "xcopy /D/S test failed rc=%u\n", rc); ok(GetFileAttributesA("xcopytest2") == INVALID_FILE_ATTRIBUTES, "xcopy copied empty directory incorrectly\n");
rc = runcmd("xcopy /D/S/E xcopytest xcopytest2\"); - todo_wine { ok(rc == 0, "xcopy /D/S/E test failed rc=%u\n", rc); + todo_wine ok(GetFileAttributesA("xcopytest2") != INVALID_FILE_ATTRIBUTES, "xcopy failed to copy empty directory\n"); - } RemoveDirectoryA("xcopytest2"); }
diff --git a/programs/xcopy/xcopy.c b/programs/xcopy/xcopy.c index 461104a8da..97b0538e57 100644 --- a/programs/xcopy/xcopy.c +++ b/programs/xcopy/xcopy.c @@ -31,12 +31,14 @@
/* * Notes: - * Apparently, valid return codes are: + * Documented valid return codes are: * 0 - OK - * 1 - No files found to copy + * 1 - No files found to copy (*1) * 2 - CTRL+C during copy * 4 - Initialization error, or invalid source specification * 5 - Disk write error + * + * (*1) Testing shows return code 1 is never returned */
@@ -1165,7 +1167,6 @@ int wmain (int argc, WCHAR *argvW[]) } else if (!(flags & OPT_NOCOPY)) { XCOPY_wprintf(XCOPY_LoadMessage(STRING_COPY), filesCopied); } - if (rc == RC_OK && filesCopied == 0) rc = RC_NOFILES; return rc;
}
Found whilst fixing bug#44967
When a directory is searched with /e, an equivalent destination directory is created immediately, not just when contents are found.
Signed-off-by: Jason Edmeades us@edmeades.me.uk --- programs/xcopy/tests/xcopy.c | 1 - programs/xcopy/xcopy.c | 13 +++++++------ 2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/programs/xcopy/tests/xcopy.c b/programs/xcopy/tests/xcopy.c index 0991d49769..94cede6f67 100644 --- a/programs/xcopy/tests/xcopy.c +++ b/programs/xcopy/tests/xcopy.c @@ -103,7 +103,6 @@ static void test_parms_syntax(void)
rc = runcmd("xcopy /D/S/E xcopytest xcopytest2\"); ok(rc == 0, "xcopy /D/S/E test failed rc=%u\n", rc); - todo_wine ok(GetFileAttributesA("xcopytest2") != INVALID_FILE_ATTRIBUTES, "xcopy failed to copy empty directory\n"); RemoveDirectoryA("xcopytest2"); diff --git a/programs/xcopy/xcopy.c b/programs/xcopy/xcopy.c index 97b0538e57..e380cde226 100644 --- a/programs/xcopy/xcopy.c +++ b/programs/xcopy/xcopy.c @@ -590,6 +590,13 @@ static int XCOPY_DoCopy(WCHAR *srcstem, WCHAR *srcspec,
/* Search 2 - do subdirs */ if (flags & OPT_RECURSIVE) { + + /* If /E is supplied, create the directory now */ + if ((flags & OPT_EMPTYDIR) && + !(flags & OPT_SIMULATE)) { + XCOPY_CreateDirectory(deststem); + } + lstrcpyW(inputpath, srcstem); lstrcatW(inputpath, wchr_star); findres = TRUE; @@ -613,12 +620,6 @@ static int XCOPY_DoCopy(WCHAR *srcstem, WCHAR *srcspec, lstrcpyW(outputpath, deststem); if (*destspec == 0x00) { lstrcatW(outputpath, finddata->cFileName); - - /* If /E is supplied, create the directory now */ - if ((flags & OPT_EMPTYDIR) && - !(flags & OPT_SIMULATE)) - XCOPY_CreateDirectory(outputpath); - lstrcatW(outputpath, wchr_slash); }
-
Jason Edmeades