[PATCH] user32: Add an exception handler to GetWindowText.
Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/user32/tests/win.c | 29 ++++++++++++++++---- dlls/user32/win.c | 59 ++++++++++++++++++++++++++++++----------- 2 files changed, 67 insertions(+), 21 deletions(-) diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 6b13d93be14..2679ea0813f 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -7923,17 +7923,36 @@ static void test_gettext(void) if (0) { r = SendMessageA( hwnd, WM_GETTEXT, 0x10, 0x1000); - ok( r == 0, "settext should return zero\n"); + ok(r == 0, "WM_GETTEXT should return zero (%ld)\n", r); r = SendMessageA( hwnd, WM_GETTEXT, 0x10000, 0); - ok( r == 0, "settext should return zero (%ld)\n", r); + ok(r == 0, "WM_GETTEXT should return zero (%ld)\n", r); r = SendMessageA( hwnd, WM_GETTEXT, 0xff000000, 0x1000); - ok( r == 0, "settext should return zero (%ld)\n", r); + ok(r == 0, "WM_GETTEXT should return zero (%ld)\n", r); r = SendMessageA( hwnd, WM_GETTEXT, 0x1000, 0xff000000); - ok( r == 0, "settext should return zero (%ld)\n", r); - } + ok(r == 0, "WM_GETTEXT should return zero (%ld)\n", r); + } + + /* GetWindowText doesn't crash */ + r = GetWindowTextA(hwnd, (LPSTR)0x10, 0x1000); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r); + r = GetWindowTextA(hwnd, (LPSTR)0x10000, 0); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r); + r = GetWindowTextA(hwnd, (LPSTR)0xff000000, 0x1000); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r); + r = GetWindowTextA(hwnd, (LPSTR)0x1000, 0xff000000); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r); + + r = GetWindowTextW(hwnd, (LPWSTR)0x10, 0x1000); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r); + r = GetWindowTextW(hwnd, (LPWSTR)0x10000, 0); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r); + r = GetWindowTextW(hwnd, (LPWSTR)0xff000000, 0x1000); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r); + r = GetWindowTextW(hwnd, (LPWSTR)0x1000, 0xff000000); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r); DestroyWindow(hwnd); } diff --git a/dlls/user32/win.c b/dlls/user32/win.c index 5e89f4c2c97..e57f291be35 100644 --- a/dlls/user32/win.c +++ b/dlls/user32/win.c @@ -35,6 +35,7 @@ #include "controls.h" #include "winerror.h" #include "wine/gdi_driver.h" +#include "wine/exception.h" #include "wine/debug.h" WINE_DEFAULT_DEBUG_CHANNEL(win); @@ -3001,22 +3002,34 @@ LONG WINAPI DECLSPEC_HOTPATCH SetWindowLongW( INT WINAPI GetWindowTextA( HWND hwnd, LPSTR lpString, INT nMaxCount ) { WCHAR *buffer; + int ret = 0; if (!lpString || nMaxCount <= 0) return 0; - if (WIN_IsCurrentProcess( hwnd )) + __TRY { - lpString[0] = 0; - return (INT)SendMessageA( hwnd, WM_GETTEXT, nMaxCount, (LPARAM)lpString ); + if (WIN_IsCurrentProcess( hwnd )) + { + lpString[0] = 0; + ret = (INT)SendMessageA( hwnd, WM_GETTEXT, nMaxCount, (LPARAM)lpString ); + } + else if ((buffer = HeapAlloc( GetProcessHeap(), 0, nMaxCount * sizeof(WCHAR) ))) + { + /* when window belongs to other process, don't send a message */ + get_server_window_text( hwnd, buffer, nMaxCount ); + if (!WideCharToMultiByte( CP_ACP, 0, buffer, -1, lpString, nMaxCount, NULL, NULL )) + lpString[nMaxCount-1] = 0; + HeapFree( GetProcessHeap(), 0, buffer ); + ret = strlen(lpString); + } + } + __EXCEPT_ALL + { + ret = 0; } + __ENDTRY - /* when window belongs to other process, don't send a message */ - if (!(buffer = HeapAlloc( GetProcessHeap(), 0, nMaxCount * sizeof(WCHAR) ))) return 0; - get_server_window_text( hwnd, buffer, nMaxCount ); - if (!WideCharToMultiByte( CP_ACP, 0, buffer, -1, lpString, nMaxCount, NULL, NULL )) - lpString[nMaxCount-1] = 0; - HeapFree( GetProcessHeap(), 0, buffer ); - return strlen(lpString); + return ret; } @@ -3049,17 +3062,31 @@ INT WINAPI InternalGetWindowText(HWND hwnd,LPWSTR lpString,INT nMaxCount ) */ INT WINAPI GetWindowTextW( HWND hwnd, LPWSTR lpString, INT nMaxCount ) { + int ret; + if (!lpString || nMaxCount <= 0) return 0; - if (WIN_IsCurrentProcess( hwnd )) + __TRY { - lpString[0] = 0; - return (INT)SendMessageW( hwnd, WM_GETTEXT, nMaxCount, (LPARAM)lpString ); + if (WIN_IsCurrentProcess( hwnd )) + { + lpString[0] = 0; + ret = (INT)SendMessageW( hwnd, WM_GETTEXT, nMaxCount, (LPARAM)lpString ); + } + else + { + /* when window belongs to other process, don't send a message */ + get_server_window_text( hwnd, lpString, nMaxCount ); + ret = lstrlenW(lpString); + } } + __EXCEPT_ALL + { + ret = 0; + } + __ENDTRY - /* when window belongs to other process, don't send a message */ - get_server_window_text( hwnd, lpString, nMaxCount ); - return lstrlenW(lpString); + return ret; } -- 2.31.1
On 10/13/21 11:41 AM, Dmitry Timoshkov wrote:
Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/user32/tests/win.c | 29 ++++++++++++++++---- dlls/user32/win.c | 59 ++++++++++++++++++++++++++++++----------- 2 files changed, 67 insertions(+), 21 deletions(-)
diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 6b13d93be14..2679ea0813f 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -7923,17 +7923,36 @@ static void test_gettext(void) if (0) { r = SendMessageA( hwnd, WM_GETTEXT, 0x10, 0x1000); - ok( r == 0, "settext should return zero\n"); + ok(r == 0, "WM_GETTEXT should return zero (%ld)\n", r);
r = SendMessageA( hwnd, WM_GETTEXT, 0x10000, 0); - ok( r == 0, "settext should return zero (%ld)\n", r); + ok(r == 0, "WM_GETTEXT should return zero (%ld)\n", r);
r = SendMessageA( hwnd, WM_GETTEXT, 0xff000000, 0x1000); - ok( r == 0, "settext should return zero (%ld)\n", r); + ok(r == 0, "WM_GETTEXT should return zero (%ld)\n", r);
r = SendMessageA( hwnd, WM_GETTEXT, 0x1000, 0xff000000); - ok( r == 0, "settext should return zero (%ld)\n", r); - } + ok(r == 0, "WM_GETTEXT should return zero (%ld)\n", r); + }
FWIW everything else in this test function (and as far as I could see most user32, or at least recently added, code) uses space-in-paren style, if you're fixing the spaces I think it should be the other way around ;) -- Rémi Bernon <rbernon(a)codeweavers.com>
Rémi Bernon <rbernon(a)codeweavers.com> wrote:
FWIW everything else in this test function (and as far as I could see most user32, or at least recently added, code) uses space-in-paren style, if you're fixing the spaces I think it should be the other way around ;)
Well, the intent was actually to fix the failure message. I've got an impression that having a space after the first parenthese while the closing one doesn't have it is a typo. If removing the first space is a problem, I can surely resend. -- Dmitry.
On 10/13/21 11:58 AM, Dmitry Timoshkov wrote:
Rémi Bernon <rbernon(a)codeweavers.com> wrote:
FWIW everything else in this test function (and as far as I could see most user32, or at least recently added, code) uses space-in-paren style, if you're fixing the spaces I think it should be the other way around ;)
Well, the intent was actually to fix the failure message. I've got an impression that having a space after the first parenthese while the closing one doesn't have it is a typo. If removing the first space is a problem, I can surely resend.
Imho inconsistent style makes code much more difficult to read, and I think we should aim to make things more consistent, not less, -at least on the module scope- and use opportunities like this to fix inconsistencies. For some modules it's hard to tell, but doing it a local basis too is better than nothing. -- Rémi Bernon <rbernon(a)codeweavers.com>
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=99950 Your paranoid android. === w1064_tsign (64 bit report) === user32: win.c:4073: Test failed: hwnd 0000000000020178/0000000000020178 message 0200 win.c:4077: Test failed: hwnd 0000000000020178/0000000000020178 message 0201 win.c:4086: Test failed: hwnd 00000000007E0272/00000000007E0272 message 0202 win.c:4089: Test failed: hwnd 00000000007E0272/00000000007E0272 message 0200 win.c:4135: Test failed: hwnd 0000000000050054/0000000000050054 message 0200 win.c:4137: Test failed: wparam 0 win.c:4148: Test failed: hwnd 0000000000050054/0000000000050054 message 0203 win.c:4152: Test failed: message 0202 available
On 10/13/21 11:41 AM, Dmitry Timoshkov wrote:
Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/user32/tests/win.c | 29 ++++++++++++++++---- dlls/user32/win.c | 59 ++++++++++++++++++++++++++++++----------- 2 files changed, 67 insertions(+), 21 deletions(-)
diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 6b13d93be14..2679ea0813f 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -7923,17 +7923,36 @@ static void test_gettext(void) if (0) { r = SendMessageA( hwnd, WM_GETTEXT, 0x10, 0x1000); - ok( r == 0, "settext should return zero\n"); + ok(r == 0, "WM_GETTEXT should return zero (%ld)\n", r);
r = SendMessageA( hwnd, WM_GETTEXT, 0x10000, 0); - ok( r == 0, "settext should return zero (%ld)\n", r); + ok(r == 0, "WM_GETTEXT should return zero (%ld)\n", r);
r = SendMessageA( hwnd, WM_GETTEXT, 0xff000000, 0x1000); - ok( r == 0, "settext should return zero (%ld)\n", r); + ok(r == 0, "WM_GETTEXT should return zero (%ld)\n", r);
r = SendMessageA( hwnd, WM_GETTEXT, 0x1000, 0xff000000); - ok( r == 0, "settext should return zero (%ld)\n", r); - } + ok(r == 0, "WM_GETTEXT should return zero (%ld)\n", r); + } + + /* GetWindowText doesn't crash */ + r = GetWindowTextA(hwnd, (LPSTR)0x10, 0x1000); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r); + r = GetWindowTextA(hwnd, (LPSTR)0x10000, 0); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r); + r = GetWindowTextA(hwnd, (LPSTR)0xff000000, 0x1000); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r); + r = GetWindowTextA(hwnd, (LPSTR)0x1000, 0xff000000); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r); + + r = GetWindowTextW(hwnd, (LPWSTR)0x10, 0x1000); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r); + r = GetWindowTextW(hwnd, (LPWSTR)0x10000, 0); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r); + r = GetWindowTextW(hwnd, (LPWSTR)0xff000000, 0x1000); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r); + r = GetWindowTextW(hwnd, (LPWSTR)0x1000, 0xff000000); + ok(r == 0, "GetWindowText should return zero (%ld)\n", r);
DestroyWindow(hwnd); } diff --git a/dlls/user32/win.c b/dlls/user32/win.c index 5e89f4c2c97..e57f291be35 100644 --- a/dlls/user32/win.c +++ b/dlls/user32/win.c @@ -35,6 +35,7 @@ #include "controls.h" #include "winerror.h" #include "wine/gdi_driver.h" +#include "wine/exception.h" #include "wine/debug.h"
WINE_DEFAULT_DEBUG_CHANNEL(win); @@ -3001,22 +3002,34 @@ LONG WINAPI DECLSPEC_HOTPATCH SetWindowLongW( INT WINAPI GetWindowTextA( HWND hwnd, LPSTR lpString, INT nMaxCount ) { WCHAR *buffer; + int ret = 0;
if (!lpString || nMaxCount <= 0) return 0;
- if (WIN_IsCurrentProcess( hwnd )) + __TRY { - lpString[0] = 0; - return (INT)SendMessageA( hwnd, WM_GETTEXT, nMaxCount, (LPARAM)lpString ); + if (WIN_IsCurrentProcess( hwnd )) + { + lpString[0] = 0; + ret = (INT)SendMessageA( hwnd, WM_GETTEXT, nMaxCount, (LPARAM)lpString ); + } + else if ((buffer = HeapAlloc( GetProcessHeap(), 0, nMaxCount * sizeof(WCHAR) ))) + { + /* when window belongs to other process, don't send a message */ + get_server_window_text( hwnd, buffer, nMaxCount ); + if (!WideCharToMultiByte( CP_ACP, 0, buffer, -1, lpString, nMaxCount, NULL, NULL )) + lpString[nMaxCount-1] = 0; + HeapFree( GetProcessHeap(), 0, buffer ); + ret = strlen(lpString); + } + } + __EXCEPT_ALL + { + ret = 0; } + __ENDTRY
- /* when window belongs to other process, don't send a message */ - if (!(buffer = HeapAlloc( GetProcessHeap(), 0, nMaxCount * sizeof(WCHAR) ))) return 0; - get_server_window_text( hwnd, buffer, nMaxCount ); - if (!WideCharToMultiByte( CP_ACP, 0, buffer, -1, lpString, nMaxCount, NULL, NULL )) - lpString[nMaxCount-1] = 0; - HeapFree( GetProcessHeap(), 0, buffer ); - return strlen(lpString); + return ret; }
@@ -3049,17 +3062,31 @@ INT WINAPI InternalGetWindowText(HWND hwnd,LPWSTR lpString,INT nMaxCount ) */ INT WINAPI GetWindowTextW( HWND hwnd, LPWSTR lpString, INT nMaxCount ) { + int ret; + if (!lpString || nMaxCount <= 0) return 0;
- if (WIN_IsCurrentProcess( hwnd )) + __TRY { - lpString[0] = 0; - return (INT)SendMessageW( hwnd, WM_GETTEXT, nMaxCount, (LPARAM)lpString ); + if (WIN_IsCurrentProcess( hwnd )) + { + lpString[0] = 0; + ret = (INT)SendMessageW( hwnd, WM_GETTEXT, nMaxCount, (LPARAM)lpString ); + } + else + { + /* when window belongs to other process, don't send a message */ + get_server_window_text( hwnd, lpString, nMaxCount ); + ret = lstrlenW(lpString); + } } + __EXCEPT_ALL + { + ret = 0; + } + __ENDTRY
- /* when window belongs to other process, don't send a message */ - get_server_window_text( hwnd, lpString, nMaxCount ); - return lstrlenW(lpString); + return ret; }
Then I'm not sure if it's good to catch all exceptions. It will swallow any error coming from the heap or send message. Is it really what it should do? -- Rémi Bernon <rbernon(a)codeweavers.com>
Rémi Bernon <rbernon(a)codeweavers.com> wrote:
Then I'm not sure if it's good to catch all exceptions. It will swallow any error coming from the heap or send message. Is it really what it should do?
I have an application that passes rubbish addresses to GetWindowText(), and apparently it works fine under Windows. -- Dmitry.
On 10/13/21 05:18, Dmitry Timoshkov wrote:
Rémi Bernon <rbernon(a)codeweavers.com> wrote:
Then I'm not sure if it's good to catch all exceptions. It will swallow any error coming from the heap or send message. Is it really what it should do?
I have an application that passes rubbish addresses to GetWindowText(), and apparently it works fine under Windows.
Should the exception be handled here, though? Is it swallowed for the cross-process case? Should it be in DefWindowProc() instead?
On 10/13/21 4:42 PM, Zebediah Figura (she/her) wrote:
On 10/13/21 05:18, Dmitry Timoshkov wrote:
Rémi Bernon <rbernon(a)codeweavers.com> wrote:
Then I'm not sure if it's good to catch all exceptions. It will swallow any error coming from the heap or send message. Is it really what it should do?
I have an application that passes rubbish addresses to GetWindowText(), and apparently it works fine under Windows.
Should the exception be handled here, though? Is it swallowed for the cross-process case? Should it be in DefWindowProc() instead?
We don't send a message for the cross process case, we call wineserver instead. Also, as far as I could see, the window proc isn't even called when an invalid string is passed, and the function still returns 0. Which matches what we are doing, as we write to the buffer once before calling SendMessageW. -- Rémi Bernon <rbernon(a)codeweavers.com>
participants (4)
-
Dmitry Timoshkov -
Marvin -
Rémi Bernon -
Zebediah Figura (she/her)