[PATCH 0/4] MR4344: Always 0-terminate output string in LoadStringW().
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/user32/resource.c | 9 +++++++-- dlls/user32/tests/resource.c | 25 +++++++++++++++++++++++++ 2 files changed, 32 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..033065d6044 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,30 @@ 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, "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, "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, "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 033065d6044..5e7aebe06a9 100644 --- a/dlls/user32/tests/resource.c +++ b/dlls/user32/tests/resource.c @@ -145,6 +145,15 @@ static void test_LoadStringW(void) ok(GetLastError() == ERROR_RESOURCE_NAME_NOT_FOUND, "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=139692 Your paranoid android. === w7u_2qxl (32 bit report) === user32: resource.c:130: Test failed: got 15100. resource.c:138: Test failed: got 15105. resource.c:145: Test failed: got 15105. === w7u_adm (32 bit report) === user32: resource.c:130: Test failed: got 15100. resource.c:138: Test failed: got 15105. resource.c:145: Test failed: got 15105. === w7pro64 (64 bit report) === user32: resource.c:130: Test failed: got 15100. resource.c:138: Test failed: got 15105. resource.c:145: Test failed: got 15105. === 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)