[PATCH] shell32: SHCreateDirectoryExW no CANCELLED for ALREADY_EXISTS
If SHCreateDirectoryExW is passed a non-null hWnd, but the directory already exists, ERROR_CANCELLED is returned, rather than ERROR_ALREADY_EXISTS An application relying on an expected return code may not proceed given the 0x4C7 1223 ERROR_CANCELLED, rather than a 0xB7 183 ERROR_ALREADY_EXISTS Warframe in-game screenshots (F6) demonstrates this behaviour Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47023 Signed-off-by: John Thomson <git(a)johnthomson.fastmail.com.au> --- My demo program of SHCreateDirectoryExW with a Hwnd returned a 183 on Windows 7, and a 1223 in wine. I would like someone else to confirm this. I am also unsure whether ERROR_FILE_EXISTS should also be returned as another exception to the rule. --- dlls/shell32/shlfileop.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c index c7bd54d200..1515ad3891 100644 --- a/dlls/shell32/shlfileop.c +++ b/dlls/shell32/shlfileop.c @@ -763,7 +763,9 @@ int WINAPI SHCreateDirectoryExW(HWND hWnd, LPCWSTR path, LPSECURITY_ATTRIBUTES s } } - if (ret && hWnd && (ERROR_CANCELLED != ret)) + if (ret && hWnd && + ret != ERROR_CANCELLED && + ret != ERROR_ALREADY_EXISTS) { /* We failed and should show a dialog box */ FIXME("Show system error message, creating path %s, failed with error %d\n", debugstr_w(path), ret); -- 2.21.0
On 4/16/19 9:57 AM, John Thomson wrote:
If SHCreateDirectoryExW is passed a non-null hWnd, but the directory already exists, ERROR_CANCELLED is returned, rather than ERROR_ALREADY_EXISTS
An application relying on an expected return code may not proceed given the 0x4C7 1223 ERROR_CANCELLED, rather than a 0xB7 183 ERROR_ALREADY_EXISTS
Warframe in-game screenshots (F6) demonstrates this behaviour
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47023 Signed-off-by: John Thomson <git(a)johnthomson.fastmail.com.au> --- My demo program of SHCreateDirectoryExW with a Hwnd returned a 183 on Windows 7, and a 1223 in wine. I would like someone else to confirm this.
I am also unsure whether ERROR_FILE_EXISTS should also be returned as another exception to the rule. --- You can write some conformance tests and verify the behavior on TestBots(https://testbot.winehq.org/)
Thanks. Zhiyi
dlls/shell32/shlfileop.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c index c7bd54d200..1515ad3891 100644 --- a/dlls/shell32/shlfileop.c +++ b/dlls/shell32/shlfileop.c @@ -763,7 +763,9 @@ int WINAPI SHCreateDirectoryExW(HWND hWnd, LPCWSTR path, LPSECURITY_ATTRIBUTES s } }
- if (ret && hWnd && (ERROR_CANCELLED != ret)) + if (ret && hWnd && + ret != ERROR_CANCELLED && + ret != ERROR_ALREADY_EXISTS) { /* We failed and should show a dialog box */ FIXME("Show system error message, creating path %s, failed with error %d\n", debugstr_w(path), ret);
On Tue, 16 Apr 2019, at 12:20, Zhiyi Zhang wrote:
You can write some conformance tests and verify the behavior on TestBots(https://testbot.winehq.org/)
Thanks! I submitted another email with just the tests, sorry for the noise. I was not sure if a reply patch would be picked up? It looks like it worked on Windows. I manually downloaded the built exe, and it failed on unpatched Wine. I wasn't able to build the Windows test executable with mingw myself. Has make crosstest changed, or it is just me?
"SHCreateDirectoryExW returned %d. Expected %d",
I missed a newline at the end of one of the ok strings Should I add this newline, squash both of my commits, and submit a new version of this patch? Any other hints or comments? Thank you! -- John Thomson
Hi John, On Tue, Apr 16, 2019 at 5:59 AM John Thomson < git(a)johnthomson.fastmail.com.au> wrote:
"SHCreateDirectoryExW returned %d. Expected %d",
I missed a newline at the end of one of the ok strings
Should I add this newline, squash both of my commits, and submit a new
version of this patch?
Any other hints or comments?
Yes, you need to send a new try. And you will need to send your fixing and tests in one patch, or at least in a same series. You can check the status of your patches in https://source.winehq.org/patches/ . https://wiki.winehq.org/Submitting_Patches -- Regards, Jactry Zeng
On 4/16/19 1:58 PM, John Thomson wrote:
On Tue, 16 Apr 2019, at 12:20, Zhiyi Zhang wrote:
You can write some conformance tests and verify the behavior on TestBots(https://testbot.winehq.org/) Thanks!
I submitted another email with just the tests, sorry for the noise. I was not sure if a reply patch would be picked up? It looks like it worked on Windows. I manually downloaded the built exe, and it failed on unpatched Wine.
I wasn't able to build the Windows test executable with mingw myself. Has make crosstest changed, or it is just me? crosstest is gone as part of the building PE dll changes.
make -C dlls/shell32/tests and copy the shell32_test.exe in dlls/shell32/tests to test it on windows. You can also submit the patch directly to Testbot, which I would recommend. Your tests failed. Please fix it.
"SHCreateDirectoryExW returned %d. Expected %d", I missed a newline at the end of one of the ok strings
Should I add this newline, squash both of my commits, and submit a new version of this patch? Any other hints or comments?
You can send a new version and add a v2 to the subject for example.
Thank you!
If SHCreateDirectoryExW is passed a non-null hWnd, but the directory already exists, ERROR_CANCELLED is returned in Wine, rather than ERROR_ALREADY_EXISTS, the Windows behaviour. An application relying on an expected return code may not proceed given the 0x4C7 1223 ERROR_CANCELLED, rather than a 0xB7 183 ERROR_ALREADY_EXISTS Warframe in-game screenshots (F6) demonstrates this behaviour Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47023 Signed-off-by: John Thomson <git(a)johnthomson.fastmail.com.au> --- v2: Add Test for conformance of SHCreateDirectoryExW with a Hwnd when target directory already exists --- dlls/shell32/shlfileop.c | 4 +++- dlls/shell32/tests/shlfileop.c | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c index c7bd54d200..1515ad3891 100644 --- a/dlls/shell32/shlfileop.c +++ b/dlls/shell32/shlfileop.c @@ -763,7 +763,9 @@ int WINAPI SHCreateDirectoryExW(HWND hWnd, LPCWSTR path, LPSECURITY_ATTRIBUTES s } } - if (ret && hWnd && (ERROR_CANCELLED != ret)) + if (ret && hWnd && + ret != ERROR_CANCELLED && + ret != ERROR_ALREADY_EXISTS) { /* We failed and should show a dialog box */ FIXME("Show system error message, creating path %s, failed with error %d\n", debugstr_w(path), ret); diff --git a/dlls/shell32/tests/shlfileop.c b/dlls/shell32/tests/shlfileop.c index 8fb4dcf66b..d43579329e 100644 --- a/dlls/shell32/tests/shlfileop.c +++ b/dlls/shell32/tests/shlfileop.c @@ -2543,6 +2543,7 @@ static void test_unicode(void) int ret; HANDLE file; static const WCHAR UNICODE_PATH_TO[] = {'c',':','\\',0x00ae,0x00ae,'\0'}; + HWND hwnd; shfoW.hwnd = NULL; shfoW.wFunc = FO_DELETE; @@ -2633,6 +2634,21 @@ static void test_unicode(void) ok(GetLastError() == ERROR_SUCCESS || broken(GetLastError() == ERROR_INVALID_HANDLE), /* WinXp, win2k3 */ "Expected ERROR_SUCCESS, got %d\n", GetLastError()); + + /* Check SHCreateDirectoryExW with a Hwnd + * returns ERROR_ALREADY_EXISTS where a directory already exists */ + /* Get any window handle */ + hwnd = FindWindowA(NULL, NULL); + ok(hwnd, "FindWindowA failed to produce a hwnd"); + ret = SHCreateDirectoryExW(hwnd, UNICODE_PATH, NULL); + ok(!ret, "SHCreateDirectoryExW returned %d\n", ret); + /* Create already-existing directory */ + ok(file_existsW(UNICODE_PATH), "The directory was not created\n"); + ret = SHCreateDirectoryExW(hwnd, UNICODE_PATH, NULL); + ok(ret == ERROR_ALREADY_EXISTS, + "SHCreateDirectoryExW returned %d. Expected ALREADY EXISTS %d\n", + ret, ERROR_ALREADY_EXISTS); + RemoveDirectoryW(UNICODE_PATH); } static void -- 2.21.0
If SHCreateDirectoryExW is passed a non-null hWnd, but the directory already exists, ERROR_CANCELLED is returned in Wine, rather than ERROR_ALREADY_EXISTS, the Windows behaviour. An application relying on an expected return code may not proceed given the 0x4C7 1223 ERROR_CANCELLED, rather than a 0xB7 183 ERROR_ALREADY_EXISTS Warframe in-game screenshots (F6) demonstrates this behaviour Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47023 Signed-off-by: John Thomson <git(a)johnthomson.fastmail.com.au> --- v2: Add Test for conformance of SHCreateDirectoryExW with a Hwnd when target directory already exists v3: Add missing newline on ok test hwnd fail string. Thanks Gijs --- dlls/shell32/shlfileop.c | 4 +++- dlls/shell32/tests/shlfileop.c | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c index c7bd54d200..1515ad3891 100644 --- a/dlls/shell32/shlfileop.c +++ b/dlls/shell32/shlfileop.c @@ -763,7 +763,9 @@ int WINAPI SHCreateDirectoryExW(HWND hWnd, LPCWSTR path, LPSECURITY_ATTRIBUTES s } } - if (ret && hWnd && (ERROR_CANCELLED != ret)) + if (ret && hWnd && + ret != ERROR_CANCELLED && + ret != ERROR_ALREADY_EXISTS) { /* We failed and should show a dialog box */ FIXME("Show system error message, creating path %s, failed with error %d\n", debugstr_w(path), ret); diff --git a/dlls/shell32/tests/shlfileop.c b/dlls/shell32/tests/shlfileop.c index 8fb4dcf66b..62f2cb3d6a 100644 --- a/dlls/shell32/tests/shlfileop.c +++ b/dlls/shell32/tests/shlfileop.c @@ -2543,6 +2543,7 @@ static void test_unicode(void) int ret; HANDLE file; static const WCHAR UNICODE_PATH_TO[] = {'c',':','\\',0x00ae,0x00ae,'\0'}; + HWND hwnd; shfoW.hwnd = NULL; shfoW.wFunc = FO_DELETE; @@ -2633,6 +2634,21 @@ static void test_unicode(void) ok(GetLastError() == ERROR_SUCCESS || broken(GetLastError() == ERROR_INVALID_HANDLE), /* WinXp, win2k3 */ "Expected ERROR_SUCCESS, got %d\n", GetLastError()); + + /* Check SHCreateDirectoryExW with a Hwnd + * returns ERROR_ALREADY_EXISTS where a directory already exists */ + /* Get any window handle */ + hwnd = FindWindowA(NULL, NULL); + ok(hwnd, "FindWindowA failed to produce a hwnd\n"); + ret = SHCreateDirectoryExW(hwnd, UNICODE_PATH, NULL); + ok(!ret, "SHCreateDirectoryExW returned %d\n", ret); + /* Create already-existing directory */ + ok(file_existsW(UNICODE_PATH), "The directory was not created\n"); + ret = SHCreateDirectoryExW(hwnd, UNICODE_PATH, NULL); + ok(ret == ERROR_ALREADY_EXISTS, + "SHCreateDirectoryExW returned %d. Expected ALREADY EXISTS %d\n", + ret, ERROR_ALREADY_EXISTS); + RemoveDirectoryW(UNICODE_PATH); } static void -- 2.21.0
If SHCreateDirectoryExW is passed a non-null hWnd, but the directory already exists, ERROR_CANCELLED is returned in Wine, rather than ERROR_ALREADY_EXISTS, the Windows behaviour. An application relying on an expected return code may not proceed given the 0x4C7 1223 ERROR_CANCELLED, rather than a 0xB7 183 ERROR_ALREADY_EXISTS Warframe in-game screenshots (F6) demonstrates this behaviour Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47023 Signed-off-by: John Thomson <git(a)johnthomson.fastmail.com.au> --- v2: Add Test for conformance of SHCreateDirectoryExW with a Hwnd when target directory already exists v3: Add missing newline on ok test hwnd fail string. Thanks Gijs v4: Made SHCreateDirectoryExW with Hwnd test already existing directory fail string consistent with existing fail strings. Thanks Gijs --- dlls/shell32/shlfileop.c | 4 +++- dlls/shell32/tests/shlfileop.c | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c index c7bd54d200..1515ad3891 100644 --- a/dlls/shell32/shlfileop.c +++ b/dlls/shell32/shlfileop.c @@ -763,7 +763,9 @@ int WINAPI SHCreateDirectoryExW(HWND hWnd, LPCWSTR path, LPSECURITY_ATTRIBUTES s } } - if (ret && hWnd && (ERROR_CANCELLED != ret)) + if (ret && hWnd && + ret != ERROR_CANCELLED && + ret != ERROR_ALREADY_EXISTS) { /* We failed and should show a dialog box */ FIXME("Show system error message, creating path %s, failed with error %d\n", debugstr_w(path), ret); diff --git a/dlls/shell32/tests/shlfileop.c b/dlls/shell32/tests/shlfileop.c index 8fb4dcf66b..474f5fd790 100644 --- a/dlls/shell32/tests/shlfileop.c +++ b/dlls/shell32/tests/shlfileop.c @@ -2543,6 +2543,7 @@ static void test_unicode(void) int ret; HANDLE file; static const WCHAR UNICODE_PATH_TO[] = {'c',':','\\',0x00ae,0x00ae,'\0'}; + HWND hwnd; shfoW.hwnd = NULL; shfoW.wFunc = FO_DELETE; @@ -2633,6 +2634,20 @@ static void test_unicode(void) ok(GetLastError() == ERROR_SUCCESS || broken(GetLastError() == ERROR_INVALID_HANDLE), /* WinXp, win2k3 */ "Expected ERROR_SUCCESS, got %d\n", GetLastError()); + + /* Check SHCreateDirectoryExW with a Hwnd + * returns ERROR_ALREADY_EXISTS where a directory already exists */ + /* Get any window handle */ + hwnd = FindWindowA(NULL, NULL); + ok(hwnd, "FindWindowA failed to produce a hwnd\n"); + ret = SHCreateDirectoryExW(hwnd, UNICODE_PATH, NULL); + ok(!ret, "SHCreateDirectoryExW returned %d\n", ret); + /* Create already-existing directory */ + ok(file_existsW(UNICODE_PATH), "The directory was not created\n"); + ret = SHCreateDirectoryExW(hwnd, UNICODE_PATH, NULL); + ok(ret == ERROR_ALREADY_EXISTS, + "Expected ERROR_ALREADY_EXISTS, got %d\n", ret); + RemoveDirectoryW(UNICODE_PATH); } static void -- 2.21.0
participants (3)
-
Jactry Zeng -
John Thomson -
Zhiyi Zhang