[PATCH v7 0/4] MR9112: XCOPY and COPY should not attempt to copy a file to itself.
Current code attempts to copy a file to itself. File is not actually copied; instead, a file sharing error is received. This result is ugly and differs from native. Behavior now appears to be the same as native with changes in this MR . -- v7: cmd/tests: Add tests for COPY to self. cmd: Don't attempt to copy a file to itself. cmd: Fix error parsing <NUL in the lexer. https://gitlab.winehq.org/wine/wine/-/merge_requests/9112
From: Joe Souza <jsouza(a)yahoo.com> --- programs/xcopy/xcopy.c | 62 ++++++++++++++++++++++++++++++++++------- programs/xcopy/xcopy.h | 1 + programs/xcopy/xcopy.rc | 1 + 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/programs/xcopy/xcopy.c b/programs/xcopy/xcopy.c index ee30e07b184..3bb00d917e5 100644 --- a/programs/xcopy/xcopy.c +++ b/programs/xcopy/xcopy.c @@ -325,6 +325,37 @@ static BOOL XCOPY_ProcessExcludeList(WCHAR* parms) { return FALSE; } +/* ========================================================================= + * XCOPY_IsSameFile + * + * Checks if the two paths reference to the same file. + * Copied from WCMD builtins.c, and tab-adjusted. + * ========================================================================= */ +static BOOL XCOPY_IsSameFile(const WCHAR *name1, const WCHAR *name2) +{ + BOOL ret = FALSE; + HANDLE file1 = INVALID_HANDLE_VALUE, file2 = INVALID_HANDLE_VALUE; + BY_HANDLE_FILE_INFORMATION info1, info2; + + file1 = CreateFileW(name1, 0, FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE, 0, OPEN_EXISTING, 0, 0); + if (file1 == INVALID_HANDLE_VALUE || !GetFileInformationByHandle(file1, &info1)) + goto end; + + file2 = CreateFileW(name2, 0, FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE, 0, OPEN_EXISTING, 0, 0); + if (file2 == INVALID_HANDLE_VALUE || !GetFileInformationByHandle(file2, &info2)) + goto end; + + ret = info1.dwVolumeSerialNumber == info2.dwVolumeSerialNumber + && info1.nFileIndexHigh == info2.nFileIndexHigh + && info1.nFileIndexLow == info2.nFileIndexLow; +end: + if (file1 != INVALID_HANDLE_VALUE) + CloseHandle(file1); + if (file2 != INVALID_HANDLE_VALUE) + CloseHandle(file2); + return ret; +} + /* ========================================================================= XCOPY_DoCopy - Recursive function to copy files based on input parms of a stem and a spec @@ -345,7 +376,7 @@ static int XCOPY_DoCopy(WCHAR *srcstem, WCHAR *srcspec, WCHAR *inputpath, *outputpath; BOOL copiedFile = FALSE; DWORD destAttribs, srcAttribs; - BOOL skipFile; + BOOL skipFile, quitCopy = FALSE; int ret = 0; /* Allocate some working memory on heap to minimize footprint */ @@ -359,7 +390,7 @@ static int XCOPY_DoCopy(WCHAR *srcstem, WCHAR *srcspec, /* Search 1 - Look for matching files */ h = FindFirstFileW(inputpath, finddata); - while (h != INVALID_HANDLE_VALUE && findres) { + while (h != INVALID_HANDLE_VALUE && !quitCopy && findres) { skipFile = FALSE; @@ -465,6 +496,18 @@ static int XCOPY_DoCopy(WCHAR *srcstem, WCHAR *srcspec, } } + /* Don't copy a file over itself. */ + if (XCOPY_IsSameFile(copyFrom, copyTo)) { + skipFile = quitCopy = TRUE; + if (!(flags & OPT_QUIET)) { + if (flags & OPT_FULL) + XCOPY_wprintf(L"%1 -> %2\n", copyFrom, copyTo); + else + XCOPY_wprintf(L"%1\n", copyFrom); + } + XCOPY_wprintf(XCOPY_LoadMessage(STRING_NOCOPYTOSELF)); + } + /* Prompt each file if necessary */ if (!skipFile && (flags & OPT_SRCPROMPT)) { DWORD count; @@ -550,11 +593,10 @@ static int XCOPY_DoCopy(WCHAR *srcstem, WCHAR *srcspec, copyFrom, copyTo, error); XCOPY_FailMessage(error); - if (flags & OPT_IGNOREERRORS) { - skipFile = TRUE; - } else { + skipFile = TRUE; + if (!(flags & OPT_IGNOREERRORS)) { ret = RC_WRITEERROR; - goto cleanup; + quitCopy = TRUE; } } else { @@ -580,12 +622,14 @@ static int XCOPY_DoCopy(WCHAR *srcstem, WCHAR *srcspec, } /* Find next file */ - findres = FindNextFileW(h, finddata); + if (!quitCopy) { + findres = FindNextFileW(h, finddata); + } } FindClose(h); /* Search 2 - do subdirs */ - if (flags & OPT_RECURSIVE) { + if (!quitCopy && (flags & OPT_RECURSIVE)) { /* If /E is supplied, create the directory now */ if ((flags & OPT_EMPTYDIR) && @@ -628,8 +672,6 @@ static int XCOPY_DoCopy(WCHAR *srcstem, WCHAR *srcspec, FindClose(h); } -cleanup: - /* free up memory */ HeapFree(GetProcessHeap(), 0, finddata); HeapFree(GetProcessHeap(), 0, inputpath); diff --git a/programs/xcopy/xcopy.h b/programs/xcopy/xcopy.h index 1ed9e20c45b..70b0935ba2c 100644 --- a/programs/xcopy/xcopy.h +++ b/programs/xcopy/xcopy.h @@ -69,3 +69,4 @@ #define STRING_FILE_CHAR 115 #define STRING_DIR_CHAR 116 #define STRING_HELP 117 +#define STRING_NOCOPYTOSELF 118 diff --git a/programs/xcopy/xcopy.rc b/programs/xcopy/xcopy.rc index c61e4da88fd..b12eeb4b068 100644 --- a/programs/xcopy/xcopy.rc +++ b/programs/xcopy/xcopy.rc @@ -79,4 +79,5 @@ Where:\n\ \t\tIf no date is supplied, only copy if destination is older\n\ \t\tthan source.\n\n" + STRING_NOCOPYTOSELF, "File cannot be copied to itself.\n" } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9112
From: Joe Souza <jsouza(a)yahoo.com> --- programs/cmd/wcmdmain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 46b89544221..103eaa486c8 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -3102,7 +3102,7 @@ static void lexer_push_command(struct node_builder *builder, } else { - filename = WCMD_parameter(p + 1, 0, NULL, FALSE, FALSE); + filename = WCMD_parameter(p, 0, NULL, FALSE, FALSE); tkn_pmt.redirection = redirection_create_file(REDIR_READ_FROM, 0, filename); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9112
From: Joe Souza <jsouza(a)yahoo.com> --- programs/cmd/builtins.c | 19 ++++++++++++++++--- programs/cmd/cmd.rc | 1 + programs/cmd/wcmd.h | 1 + 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index ef84bee64ac..f9a286dc92a 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -672,6 +672,7 @@ RETURN_CODE WCMD_copy(WCHAR * args) DWORD len; BOOL dstisdevice = FALSE; unsigned numcopied = 0; + BOOL want_numcopied = FALSE; typedef struct _COPY_FILES { @@ -1041,6 +1042,7 @@ RETURN_CODE WCMD_copy(WCHAR * args) WCHAR outname[MAX_PATH]; BOOL overwrite; BOOL appendtofirstfile = FALSE; + BOOL issamefile; /* Skip . and .., and directories */ if (!srcisdevice && fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { @@ -1060,13 +1062,24 @@ RETURN_CODE WCMD_copy(WCHAR * args) overwrite = TRUE; } + issamefile = WCMD_IsSameFile(srcpath, outname); + WINE_TRACE("Copying from : '%s'\n", wine_dbgstr_w(srcpath)); WINE_TRACE("Copying to : '%s'\n", wine_dbgstr_w(outname)); WINE_TRACE("Flags: srcbinary(%d), dstbinary(%d), over(%d), prompt(%d)\n", thiscopy->binarycopy, destination->binarycopy, overwrite, prompt); + if (!anyconcats && issamefile) { + WCMD_output_asis(srcpath); + WCMD_output_asis(L"\r\n"); + WCMD_output_stderr(WCMD_LoadMessage(WCMD_NOCOPYTOSELF)); + return_code = ERROR_INVALID_FUNCTION; + want_numcopied = TRUE; + break; + } + if (!writtenoneconcat) { - appendtofirstfile = anyconcats && WCMD_IsSameFile(srcpath, outname); + appendtofirstfile = anyconcats && issamefile; } /* Prompt before overwriting */ @@ -1098,7 +1111,7 @@ RETURN_CODE WCMD_copy(WCHAR * args) WCMD_output_asis(srcpath); WCMD_output_asis(L"\r\n"); } - if (anyconcats && WCMD_IsSameFile(srcpath, outname)) { + if (anyconcats && issamefile) { /* behavior is as Unix 'touch' (change last-written time only) */ HANDLE file = CreateFileW(srcpath, GENERIC_WRITE, FILE_SHARE_WRITE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); @@ -1168,7 +1181,7 @@ RETURN_CODE WCMD_copy(WCHAR * args) } } - if (numcopied) { + if (numcopied || want_numcopied) { WCMD_output(WCMD_LoadMessage(WCMD_NUMCOPIED), numcopied); } diff --git a/programs/cmd/cmd.rc b/programs/cmd/cmd.rc index 29d105fa7a2..322673aab30 100644 --- a/programs/cmd/cmd.rc +++ b/programs/cmd/cmd.rc @@ -407,4 +407,5 @@ Enter HELP <command> for further information on any of the above commands.\n" WCMD_ENDOFLINE, "End of line" WCMD_ENDOFFILE, "End of file" WCMD_NUMCOPIED, "%t%1!u! file(s) copied\n" + WCMD_NOCOPYTOSELF, "File cannot be copied to itself.\n" } diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h index 6810598142c..ba18b942c5d 100644 --- a/programs/cmd/wcmd.h +++ b/programs/cmd/wcmd.h @@ -477,3 +477,4 @@ extern WCHAR version_string[]; #define WCMD_ENDOFLINE 1048 #define WCMD_ENDOFFILE 1049 #define WCMD_NUMCOPIED 1050 +#define WCMD_NOCOPYTOSELF 1051 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9112
From: Joe Souza <jsouza(a)yahoo.com> --- programs/cmd/tests/test_builtins.bat | 6 ++++++ programs/cmd/tests/test_builtins.bat.exp | 6 ++++++ programs/cmd/tests/test_builtins.cmd | 6 ++++++ programs/cmd/tests/test_builtins.cmd.exp | 6 ++++++ 4 files changed, 24 insertions(+) diff --git a/programs/cmd/tests/test_builtins.bat b/programs/cmd/tests/test_builtins.bat index dc624111213..e36cc742053 100644 --- a/programs/cmd/tests/test_builtins.bat +++ b/programs/cmd/tests/test_builtins.bat @@ -114,6 +114,12 @@ call :setError 666 & (copy fileA fileZ /-Y >NUL <NUL &&echo SUCCESS !errorlevel! call :setError 666 & (copy fileA+fileD fileZ /-Y >NUL <NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & (copy fileD+fileA fileZ /-Y >NUL <NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) if exist fileD echo Unexpected fileD +call :setError 666 & (copy /b fileA fileA /Y >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (copy /b fileA+fileB fileA >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +type fileA +echo a > fileA +call :setError 666 & (copy /b fileA+fileB /Y fileB >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +type fileB cd .. && rd /q /s foo echo --- success/failure for MOVE command diff --git a/programs/cmd/tests/test_builtins.bat.exp b/programs/cmd/tests/test_builtins.bat.exp index d3f23abde77..6dbd418a8f5 100644 --- a/programs/cmd/tests/test_builtins.bat.exp +++ b/programs/cmd/tests/test_builtins.bat.exp @@ -77,6 +77,12 @@ SUCCESS 0 FAILURE 1 FAILURE 1 FAILURE 1 +FAILURE 1 +SUCCESS 0 +a +b +SUCCESS 0 +a --- success/failure for MOVE command FAILURE 1 SUCCESS 0 diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 19dc3c788bb..11d92dd16c4 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -639,6 +639,12 @@ call :setError 666 & (copy fileA fileZ /-Y >NUL <NUL &&echo SUCCESS !errorlevel! call :setError 666 & (copy fileA+fileD fileZ /-Y >NUL <NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & (copy fileD+fileA fileZ /-Y >NUL <NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) if exist fileD echo Unexpected fileD +call :setError 666 & (copy /b fileA fileA /Y >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (copy /b fileA+fileB fileA >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +type fileA +echo a > fileA +call :setError 666 & (copy /b fileA+fileB /Y fileB >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +type fileB cd .. && rd /q /s foo echo --- success/failure for MOVE command diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 644ca910c8c..38bb41e181f 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -541,6 +541,12 @@ SUCCESS 0 FAILURE 1 FAILURE 1 FAILURE 1 +FAILURE 1 +SUCCESS 0 +a +b +SUCCESS 0 +a --- success/failure for MOVE command FAILURE 1 SUCCESS 0 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9112
On Thu Oct 23 19:57:57 2025 +0000, eric pouech wrote:
this change looks very dubious Fixed in latest change.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9112#note_119499
Latest changes pushed. No todo_wine because the results are the same with or without my changes. Otherwise have integrated all of your feedback. Still need to add xcopy tests. That will happen later. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9112#note_119501
we don't get todo!s as 'copy a a' ends up with a sharing violation in CopyFileW, which is caght, reported, and ends up with 1 as return_code (same value as the correct detection) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9112#note_119558
participants (3)
-
eric pouech (＠epo) -
Joe Souza
-
Joe Souza (＠JoeS209)