[PATCH v2 0/4] MR4344: Always 0-terminate output string in LoadStringW().
-- v2: kernelbase: Put 0 to output string even for 1 char buffer in LoadStringW(). kernelbase: Return empty string from LoadStringW() if resource is not found. user32: Put 0 to output string even for 1 char buffer in LoadStringW(). user32: Return empty string from LoadStringW() if resource is not found. https://gitlab.winehq.org/wine/wine/-/merge_requests/4344
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/user32/resource.c | 9 +++++++-- dlls/user32/tests/resource.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/dlls/user32/resource.c b/dlls/user32/resource.c index 3714b05ce8d..a634d79948b 100644 --- a/dlls/user32/resource.c +++ b/dlls/user32/resource.c @@ -164,9 +164,9 @@ INT WINAPI DECLSPEC_HOTPATCH LoadStringW( HINSTANCE instance, UINT resource_id, /* Use loword (incremented by 1) as resourceid */ hrsrc = FindResourceW( instance, MAKEINTRESOURCEW((LOWORD(resource_id) >> 4) + 1), (LPWSTR)RT_STRING ); - if (!hrsrc) return 0; + if (!hrsrc) goto error; hmem = LoadResource( instance, hrsrc ); - if (!hmem) return 0; + if (!hmem) goto error; p = LockResource(hmem); string_num = resource_id & 0x000f; @@ -196,6 +196,11 @@ INT WINAPI DECLSPEC_HOTPATCH LoadStringW( HINSTANCE instance, UINT resource_id, TRACE("%s loaded !\n", debugstr_w(buffer)); return i; + +error: + TRACE( "Failed to load string.\n" ); + if (buflen > 0) buffer[0] = 0; + return 0; } /********************************************************************** diff --git a/dlls/user32/tests/resource.c b/dlls/user32/tests/resource.c index 2369aef2a09..dad2cbd4a7f 100644 --- a/dlls/user32/tests/resource.c +++ b/dlls/user32/tests/resource.c @@ -70,6 +70,7 @@ static void test_LoadStringW(void) win_skip( "LoadStringW does not return a pointer to the resource\n" ); return; } + length2 = LoadStringW(hInst, 2, returnedstringw, ARRAY_SIZE(returnedstringw)); /* get resource string */ ok(length2 > 0, "LoadStringW failed to load resource 2, ret %d, err %ld\n", length2, GetLastError()); ok(length1 == length2, "LoadStringW returned different values dependent on buflen. ret1 %d, ret2 %d\n", @@ -120,6 +121,33 @@ static void test_LoadStringW(void) winetest_pop_context(); } } + + /* Test missing resource. */ + SetLastError(0xdeadbeef); + memset(returnedstringw, 0xcc, sizeof(returnedstringw)); + length1 = LoadStringW(hInst, 0xdeadbeef, returnedstringw, ARRAY_SIZE(returnedstringw)); + ok(!length1, "got %d.\n", length1); + ok(GetLastError() == ERROR_RESOURCE_NAME_NOT_FOUND || broken(GetLastError() == ERROR_MUI_FILE_NOT_FOUND) /* Win7 */, + "got %lu.\n", GetLastError()); + ok(!returnedstringw[0], "got %#x.\n", returnedstringw[0]); + ok(returnedstringw[1] == 0xcccc, "got %#x.\n", returnedstringw[1]); + + SetLastError(0xdeadbeef); + memset(returnedstringw, 0xcc, sizeof(returnedstringw)); + length1 = LoadStringW(hInst, 0xdeadbeef, returnedstringw, 0); + ok(!length1, "got %d.\n", length1); + ok(GetLastError() == ERROR_RESOURCE_NAME_NOT_FOUND || broken(GetLastError() == ERROR_MUI_FILE_NOT_LOADED) /* Win7 */, + "got %lu.\n", GetLastError()); + ok(returnedstringw[0] == 0xcccc, "got %#x.\n", returnedstringw[1]); + + SetLastError(0xdeadbeef); + memset(returnedstringw, 0xcc, sizeof(returnedstringw)); + length1 = LoadStringW(hInst, 0xdeadbeef, returnedstringw, 1); + ok(!length1, "got %d.\n", length1); + ok(GetLastError() == ERROR_RESOURCE_NAME_NOT_FOUND || broken(GetLastError() == ERROR_MUI_FILE_NOT_LOADED) /* Win7 */, + "got %lu.\n", GetLastError()); + ok(!returnedstringw[0], "got %#x.\n", returnedstringw[0]); + ok(returnedstringw[1] == 0xcccc, "got %#x.\n", returnedstringw[1]); } static void test_LoadStringA (void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4344
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/user32/resource.c | 6 +----- dlls/user32/tests/resource.c | 9 +++++++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/dlls/user32/resource.c b/dlls/user32/resource.c index a634d79948b..c09ee9bb65c 100644 --- a/dlls/user32/resource.c +++ b/dlls/user32/resource.c @@ -187,12 +187,8 @@ INT WINAPI DECLSPEC_HOTPATCH LoadStringW( HINSTANCE instance, UINT resource_id, if (i > 0) { memcpy(buffer, p + 1, i * sizeof (WCHAR)); buffer[i] = 0; - } else { - if (buflen > 1) { - buffer[0] = 0; - return 0; - } } + else goto error; TRACE("%s loaded !\n", debugstr_w(buffer)); return i; diff --git a/dlls/user32/tests/resource.c b/dlls/user32/tests/resource.c index dad2cbd4a7f..e9652b60763 100644 --- a/dlls/user32/tests/resource.c +++ b/dlls/user32/tests/resource.c @@ -148,6 +148,15 @@ static void test_LoadStringW(void) "got %lu.\n", GetLastError()); ok(!returnedstringw[0], "got %#x.\n", returnedstringw[0]); ok(returnedstringw[1] == 0xcccc, "got %#x.\n", returnedstringw[1]); + + /* Test short buffer */ + SetLastError(0xdeadbeef); + memset(returnedstringw, 0xcc, sizeof(returnedstringw)); + length1 = LoadStringW(hInst, 2, returnedstringw, 1); /* get resource string */ + ok(!length1, "got %d.\n", length1); + ok(GetLastError() == 0xdeadbeef, "got %lu.\n", GetLastError()); + ok(!returnedstringw[0], "got %#x.\n", returnedstringw[0]); + ok(returnedstringw[1] == 0xcccc, "got %#x.\n", returnedstringw[1]); } static void test_LoadStringA (void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4344
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/kernelbase/string.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/kernelbase/string.c b/dlls/kernelbase/string.c index 798bc1f3d63..1a792dcadb1 100644 --- a/dlls/kernelbase/string.c +++ b/dlls/kernelbase/string.c @@ -1231,9 +1231,9 @@ INT WINAPI DECLSPEC_HOTPATCH LoadStringW(HINSTANCE instance, UINT resource_id, L /* Use loword (incremented by 1) as resourceid */ hrsrc = FindResourceW(instance, MAKEINTRESOURCEW((LOWORD(resource_id) >> 4) + 1), (LPWSTR)RT_STRING); - if (!hrsrc) return 0; + if (!hrsrc) goto error; hmem = LoadResource(instance, hrsrc); - if (!hmem) return 0; + if (!hmem) goto error; p = LockResource(hmem); string_num = resource_id & 0x000f; @@ -1267,6 +1267,11 @@ INT WINAPI DECLSPEC_HOTPATCH LoadStringW(HINSTANCE instance, UINT resource_id, L TRACE("returning %s\n", debugstr_w(buffer)); return i; + +error: + TRACE( "Failed to load string.\n" ); + if (buflen > 0) buffer[0] = 0; + return 0; } INT WINAPI DECLSPEC_HOTPATCH LoadStringA(HINSTANCE instance, UINT resource_id, LPSTR buffer, INT buflen) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4344
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/kernelbase/string.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/dlls/kernelbase/string.c b/dlls/kernelbase/string.c index 1a792dcadb1..1df1f54aafd 100644 --- a/dlls/kernelbase/string.c +++ b/dlls/kernelbase/string.c @@ -1256,14 +1256,7 @@ INT WINAPI DECLSPEC_HOTPATCH LoadStringW(HINSTANCE instance, UINT resource_id, L memcpy(buffer, p + 1, i * sizeof (WCHAR)); buffer[i] = 0; } - else - { - if (buflen > 1) - { - buffer[0] = 0; - return 0; - } - } + else goto error; TRACE("returning %s\n", debugstr_w(buffer)); return i; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4344
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=139697 Your paranoid android. === debian11b (64 bit WoW report) === uxtheme: system.c:572: Test succeeded inside todo block: OpenThemeData() should fail system.c:574: Test succeeded inside todo block: Got unexpected 0x80070490.
participants (3)
-
Marvin -
Paul Gofman -
Paul Gofman (＠gofman)