Re: [PATCH] user32: Add an exception handler to GetWindowText.

[Rémi Bernon]

On 10/13/21 11:41 AM, Dmitry Timoshkov wrote:
> Signed-off-by: Dmitry Timoshkov
>
> 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?