[PATCH 0/1] MR7290: Draft: kernelbase: Handle correctly paths with forward slashes in ReplaceFileW.
PathRemoveFileSpecW keeps only the drive if the path contains all forward slashes as shown in tests. But then the temporary file is created in the root folder which fails for drive Z:. This fixes a corner case when using CMake in Wine. Another usage of PathRemoveFileSpecW is in PathRelativePathToW which should be also checked. I'm leaving this as a draft until getting some feedback and adding tests. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7290
From: Roman Pišl <rpisl(a)seznam.cz> PathRemoveFileSpecW keeps only the drive if the path contains all forward slashes as shown in tests. But then the temporary file is created in the root folder which fails for drive Z:. --- dlls/kernelbase/file.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/dlls/kernelbase/file.c b/dlls/kernelbase/file.c index f0dedfe3b14..53286f1331a 100644 --- a/dlls/kernelbase/file.c +++ b/dlls/kernelbase/file.c @@ -2717,8 +2717,16 @@ BOOL WINAPI DECLSPEC_HOTPATCH ReplaceFileW( const WCHAR *replaced, const WCHAR * * it out of the way first. */ WCHAR temp_path[MAX_PATH], temp_file[MAX_PATH]; - lstrcpynW( temp_path, replaced, ARRAY_SIZE( temp_path ) ); - PathRemoveFileSpecW( temp_path ); + WCHAR* filePart; + DWORD cnt = GetFullPathNameW(replaced, ARRAY_SIZE( temp_path ), temp_path, &filePart); + if (!cnt) return FALSE; + if (cnt >= ARRAY_SIZE( temp_path ) || !filePart) + { + SetLastError( ERROR_PATH_NOT_FOUND ); + return FALSE; + } + *filePart = 0; + if (!GetTempFileNameW( temp_path, L"rf", 0, temp_file ) || !MoveFileExW( replaced, temp_file, MOVEFILE_REPLACE_EXISTING )) return FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7290
I didn't find a simple and reliable way how to test this internal behavior. I'm marking it as ready. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7290#note_97179
On Sat Mar 8 14:18:34 2025 +0000, Roman Pišl wrote:
I didn't find a simple and reliable way how to test this internal behavior. I'm marking it as ready. If CMake could trigger it, it surely isn't an internal behavior. Internal behavior is something that cannot be observed by an application.
You should add a test that demonstrates what CMake did that made it run into the bug, not test for the internals directly. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7290#note_97188
On Sat Mar 8 14:19:20 2025 +0000, Jinoh Kang wrote:
If CMake could trigger it, it surely isn't an internal behavior. Internal behavior is something that cannot be observed by an application. You should add a test that demonstrates what CMake did that made it run into the bug, not test for the internals directly. But how to do it in an elegant way? This can be triggered simply in Wine on driver Z: (as / is obviously not writable for a user in Linux) but is not triggered in Wine on C: which is writable in Wine but not on Windows and I expect is the only drive on testbots.
So idea - test on Z: and fallback to C: if Z: is not present? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7290#note_97189
On Sat Mar 8 16:09:36 2025 +0000, Roman Pišl wrote:
But how to do it in an elegant way? This can be triggered simply in Wine on driver Z: (as / is obviously not writable for a user in Linux) but is not triggered in Wine on C: which is writable in Wine but not on Windows and I expect is the only drive on testbots. So idea - test on Z: and fallback to C: if Z: is not present? Another option would be to create the same conditions locally in a temporary folder, but then I'm leaving this for the long winter evenings..
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7290#note_97191
participants (2)
-
Jinoh Kang (＠iamahuman) -
Roman Pišl