[PATCH] user32: Fix error handling in MapWindowPoints, ClientToScreen and ScreenToClient and add tests for them. (try 3)
Christian Costa
titan.costa at gmail.com
Tue Oct 23 14:29:17 CDT 2012
Based ont a patch of Rico Schuller.
Try 3:
- Check window handles in WINPOS_GetWinOffset which now return an error.
Note that I was forced to use IsWindow because get_windows_offset does not return any error.
Try 2:
- Test window handle in ClienToScreen and ScreenToClient instead of relying on GetLastError().
- Don't rename params.
---
dlls/user32/tests/win.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++
dlls/user32/winpos.c | 74 ++++++++++++++--
2 files changed, 276 insertions(+), 11 deletions(-)
diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c
index e84824d..8c5dcc2 100644
--- a/dlls/user32/tests/win.c
+++ b/dlls/user32/tests/win.c
@@ -7104,6 +7104,217 @@ todo_wine
ok(ret, "UnregisterClass(my_window) failed\n");
}
+static void test_map_points(void)
+{
+ BOOL ret;
+ POINT p;
+ HWND wnd, dwnd;
+ INT n;
+ DWORD err;
+ POINT pos = { 100, 200 };
+ int width = 150;
+ int height = 150;
+ RECT window_rect;
+ RECT client_rect;
+
+ /* Create test windows */
+ wnd = CreateWindow("static", "test1", WS_POPUP, pos.x, pos.y, width, height, NULL, NULL, NULL, NULL);
+ ok(wnd != NULL, "Failed %p\n", wnd);
+ dwnd = CreateWindow("static", "test2", 0, 200, 300, 150, 150, NULL, NULL, NULL, NULL);
+ DestroyWindow(dwnd);
+ ok(dwnd != NULL, "Failed %p\n", dwnd);
+
+ /* Verify window rect and client rect (they should have the same width and height) */
+ GetWindowRect(wnd, &window_rect);
+ ok(window_rect.left == pos.x, "left is %d instead of %d\n", window_rect.left, pos.x);
+ ok(window_rect.top == pos.y, "top is %d instead of %d\n", window_rect.top, pos.y);
+ ok(window_rect.right == pos.x + width, "right is %d instead of %d\n", window_rect.right, pos.x + width);
+ ok(window_rect.bottom == pos.y + height, "bottom is %d instead of %d\n", window_rect.bottom, pos.y + height);
+ GetClientRect(wnd, &client_rect);
+ ok(client_rect.left == 0, "left is %d instead of 0\n", client_rect.left);
+ ok(client_rect.top == 0, "top is %d instead of 0\n", client_rect.top);
+ ok(client_rect.right == width, "right is %d instead of %d\n", client_rect.right, width);
+ ok(client_rect.bottom == height, "bottom is %d instead of %d\n", client_rect.bottom, height);
+
+ /* Test MapWindowPoints */
+
+ /* MapWindowPoints(NULL or wnd, NULL or wnd, NULL, 1); crashes on XP */
+
+ SetLastError(0xdeadbeef);
+ n = MapWindowPoints(NULL, NULL, NULL, 0);
+ err = GetLastError();
+ ok(n == 0, "Got %d, expected %d\n", n, 0);
+ ok(err == 0xdeadbeef, "Got %x, expected %x\n", err, 0xdeadbeef);
+
+ SetLastError(0xdeadbeef);
+ n = MapWindowPoints(wnd, wnd, NULL, 0);
+ err = GetLastError();
+ ok(n == 0, "Got %d, expected %d\n", n, 0);
+ ok(err == 0xdeadbeef, "Got %x, expected %x\n", err, 0xdeadbeef);
+
+ SetLastError(0xdeadbeef);
+ n = MapWindowPoints(wnd, NULL, NULL, 0);
+ err = GetLastError();
+ ok(n == ((window_rect.top << 16) | window_rect.left), "Got %x (%d, %d), expected %x (%d, %d)\n",
+ n, (n << 16) >> 16, n >> 16, window_rect.left, window_rect.top, (window_rect.top << 16) | window_rect.left);
+ ok(err == 0xdeadbeef, "Got %x, expected %x\n", err, 0xdeadbeef);
+
+ SetLastError(0xdeadbeef);
+ n = MapWindowPoints(NULL, wnd, NULL, 0);
+ err = GetLastError();
+ ok(n == ((-window_rect.top << 16) | (-window_rect.left & 0xffff)), "Got %x (%d, %d), expected %x (%d, %d)\n",
+ n, (n << 16) >> 16, n >> 16, (-window_rect.top << 16) | (-window_rect.left & 0xffff), -window_rect.left, -window_rect.top);
+ ok(err == 0xdeadbeef, "Got %x, expected %x\n", err, 0xdeadbeef);
+
+ SetLastError(0xdeadbeef);
+ p.x = p.y = 100;
+ n = MapWindowPoints(dwnd, NULL, &p, 1);
+ err = GetLastError();
+ ok(n == 0, "Got %d, expected %d\n", n, 0);
+ ok(p.x == 100 && p.y == 100, "Failed got(%d, %d), expected (%d, %d)\n", p.x, p.y, 100, 100);
+ ok(err == ERROR_INVALID_WINDOW_HANDLE, "Got %x, expected %x\n", err, ERROR_INVALID_WINDOW_HANDLE);
+
+ SetLastError(0xdeadbeef);
+ p.x = p.y = 100;
+ n = MapWindowPoints(dwnd, wnd, &p, 1);
+ err = GetLastError();
+ ok(n == 0, "Got %d, expected %d\n", n, 0);
+ ok(p.x == 100 && p.y == 100, "Failed got(%d, %d), expected (%d, %d)\n", p.x, p.y, 100, 100);
+ ok(err == ERROR_INVALID_WINDOW_HANDLE, "Got %x, expected %x\n", err, ERROR_INVALID_WINDOW_HANDLE);
+
+ SetLastError(0xdeadbeef);
+ p.x = p.y = 100;
+ n = MapWindowPoints(NULL, dwnd, &p, 1);
+ err = GetLastError();
+ ok(n == 0, "Got %d, expected %d\n", n, 0);
+ ok(p.x == 100 && p.y == 100, "Failed got(%d, %d), expected (%d, %d)\n", p.x, p.y, 100, 100);
+ ok(err == ERROR_INVALID_WINDOW_HANDLE, "Got %x, expected %x\n", err, ERROR_INVALID_WINDOW_HANDLE);
+
+ SetLastError(0xdeadbeef);
+ p.x = p.y = 100;
+ n = MapWindowPoints(wnd, dwnd, &p, 1);
+ err = GetLastError();
+ ok(n == 0, "Got %d, expected %d\n", n, 0);
+ ok(p.x == 100 && p.y == 100, "Failed got(%d, %d), expected (%d, %d)\n", p.x, p.y, 100, 100);
+ ok(err == ERROR_INVALID_WINDOW_HANDLE, "Got %x, expected %x\n", err, ERROR_INVALID_WINDOW_HANDLE);
+
+ SetLastError(0xdeadbeef);
+ p.x = p.y = 100;
+ n = MapWindowPoints(dwnd, dwnd, &p, 1);
+ err = GetLastError();
+ ok(n == 0, "Got %d, expected %d\n", n, 0);
+ ok(p.x == 100 && p.y == 100, "Failed got(%d, %d), expected (%d, %d)\n", p.x, p.y, 100, 100);
+ ok(err == ERROR_INVALID_WINDOW_HANDLE, "Got %x, expected %x\n", err, ERROR_INVALID_WINDOW_HANDLE);
+
+ SetLastError(0xdeadbeef);
+ p.x = p.y = 100;
+ n = MapWindowPoints(NULL, NULL, &p, 1);
+ err = GetLastError();
+ ok(n == 0, "Got %d, expected %d\n", n, 0);
+ ok(p.x == 100 && p.y == 100, "Failed got(%d, %d), expected (%d, %d)\n", p.x, p.y, 100, 100);
+ ok(err == 0xdeadbeef, "Got %x, expected %x\n", err, 0xdeadbeef);
+
+ SetLastError(0xdeadbeef);
+ p.x = p.y = 100;
+ n = MapWindowPoints(wnd, wnd, &p, 1);
+ err = GetLastError();
+ ok(n == 0, "Got %d, expected %d\n", n, 0);
+ ok(p.x == 100 && p.y == 100, "Failed got(%d, %d), expected (%d, %d)\n", p.x, p.y, 100, 100);
+ ok(err == 0xdeadbeef, "Got %x, expected %x\n", err, 0xdeadbeef);
+
+ SetLastError(0xdeadbeef);
+ p.x = p.y = 100;
+ n = MapWindowPoints(wnd, NULL, &p, 1);
+ err = GetLastError();
+ ok(n == ((window_rect.top << 16) | window_rect.left), "Got %x (%d, %d), expected %x (%d, %d)\n",
+ n, (n << 16) >> 16, n >> 16, window_rect.left, window_rect.top, (window_rect.top << 16) | window_rect.left);
+ ok((p.x == (window_rect.left + 100)) && (p.y == (window_rect.top + 100)), "Failed got (%d, %d), expected (%d, %d)\n",
+ p.x, p.y, window_rect.left + 100, window_rect.top + 100);
+ ok(err == 0xdeadbeef, "Got %x, expected %x\n", err, 0xdeadbeef);
+
+ SetLastError(0xdeadbeef);
+ p.x = p.y = 100;
+ n = MapWindowPoints(NULL, wnd, &p, 1);
+ err = GetLastError();
+ ok(n == ((-window_rect.top << 16) | (-window_rect.left & 0xffff)), "Got %x (%d, %d), expected %x (%d, %d)\n",
+ n, (n << 16) >> 16, n >> 16, (-window_rect.top << 16) | (-window_rect.left & 0xffff), -window_rect.left, -window_rect.top);
+ ok((p.x == (-window_rect.left + 100)) && (p.y == (-window_rect.top + 100)), "Failed got (%d, %d), expected (%d, %d)\n",
+ p.x, p.y, -window_rect.left + 100, -window_rect.top + 100);
+ ok(err == 0xdeadbeef, "Got %x, expected %x\n", err, 0xdeadbeef);
+
+ /* Test ClientToScreen */
+
+ /* ClientToScreen(wnd, NULL); crashes on XP */
+
+ SetLastError(0xdeadbeef);
+ ret = ClientToScreen(NULL, NULL);
+ err = GetLastError();
+ ok(!ret, "Failed\n");
+ ok(err == ERROR_INVALID_WINDOW_HANDLE, "Got %x, expected %x\n", err, ERROR_INVALID_WINDOW_HANDLE);
+
+ SetLastError(0xdeadbeef);
+ p.x = p.y = 100;
+ ret = ClientToScreen(NULL, &p);
+ err = GetLastError();
+ ok(!ret, "Failed\n");
+ ok(p.x == 100 && p.y == 100, "Failed got(%d, %d), expected (%d, %d)\n", p.x, p.y, 100, 100);
+ ok(err == ERROR_INVALID_WINDOW_HANDLE, "Got %x, expected %x\n", err, ERROR_INVALID_WINDOW_HANDLE);
+
+ SetLastError(0xdeadbeef);
+ p.x = p.y = 100;
+ ret = ClientToScreen(dwnd, &p);
+ err = GetLastError();
+ ok(!ret, "Failed\n");
+ ok(p.x == 100 && p.y == 100, "Failed got(%d, %d), expected (%d, %d)\n", p.x, p.y, 100, 100);
+ ok(err == ERROR_INVALID_WINDOW_HANDLE, "Got %x, expected %x\n", err, ERROR_INVALID_WINDOW_HANDLE);
+
+ SetLastError(0xdeadbeef);
+ p.x = p.y = 100;
+ ret = ClientToScreen(wnd, &p);
+ err = GetLastError();
+ ok(ret, "Failed\n");
+ ok((p.x == (window_rect.left + 100)) && (p.y == (window_rect.top + 100)), "Failed got (%d, %d), expected (%d, %d)\n",
+ p.x, p.y, window_rect.left + 100, window_rect.top + 100);
+ ok(err == 0xdeadbeef, "Got %x, expected %x\n", err, 0xdeadbeef);
+
+ /* Test ScreenToClient */
+
+ /* ScreenToClient(wnd, NULL); crashes on XP */
+
+ SetLastError(0xdeadbeef);
+ ret = ScreenToClient(NULL, NULL);
+ err = GetLastError();
+ ok(!ret, "Failed\n");
+ ok(err == ERROR_INVALID_WINDOW_HANDLE, "Got %x, expected %x\n", err, ERROR_INVALID_WINDOW_HANDLE);
+
+ SetLastError(0xdeadbeef);
+ p.x = p.y = 100;
+ ret = ScreenToClient(NULL, &p);
+ err = GetLastError();
+ ok(!ret, "Failed\n");
+ ok(p.x == 100 && p.y == 100, "Failed got(%d, %d), expected (%d, %d)\n", p.x, p.y, 100, 100);
+ ok(err == ERROR_INVALID_WINDOW_HANDLE, "Got %x, expected %x\n", err, ERROR_INVALID_WINDOW_HANDLE);
+
+ SetLastError(0xdeadbeef);
+ p.x = p.y = 100;
+ ret = ScreenToClient(dwnd, &p);
+ err = GetLastError();
+ ok(ret == FALSE, "Failed\n");
+ ok(p.x == 100 && p.y == 100, "Failed got(%d, %d), expected (%d, %d)\n", p.x, p.y, 100, 100);
+ ok(err == ERROR_INVALID_WINDOW_HANDLE, "Got %x, expected %x\n", err, ERROR_INVALID_WINDOW_HANDLE);
+
+ SetLastError(0xdeadbeef);
+ p.x = p.y = 100;
+ ret = ScreenToClient(wnd, &p);
+ err = GetLastError();
+ ok(ret, "Failed\n");
+ ok((p.x == (-window_rect.left + 100)) && (p.y == (-window_rect.top + 100)), "Failed got(%d, %d), expected (%d, %d)\n",
+ p.x, p.y, -window_rect.left + 100, -window_rect.top + 100);
+ ok(err == 0xdeadbeef, "Got %x, expected %x\n", err, 0xdeadbeef);
+
+ DestroyWindow(wnd);
+}
+
START_TEST(win)
{
HMODULE user32 = GetModuleHandleA( "user32.dll" );
@@ -7213,6 +7424,8 @@ START_TEST(win)
test_handles( hwndMain );
test_winregion();
+ test_map_points();
+
/* add the tests above this line */
if (hhook) UnhookWindowsHookEx(hhook);
diff --git a/dlls/user32/winpos.c b/dlls/user32/winpos.c
index 5a2f1f2..531fae11 100644
--- a/dlls/user32/winpos.c
+++ b/dlls/user32/winpos.c
@@ -236,21 +236,49 @@ BOOL WINAPI GetClientRect( HWND hwnd, LPRECT rect )
/*******************************************************************
- * ClientToScreen (USER32.@)
+ * ClientToScreen (USER32.@)
*/
BOOL WINAPI ClientToScreen( HWND hwnd, LPPOINT lppnt )
{
+ DWORD error = GetLastError();
+
+ if (!hwnd)
+ {
+ SetLastError( ERROR_INVALID_WINDOW_HANDLE );
+ return FALSE;
+ }
+
+ SetLastError( 0xdeadbeef );
MapWindowPoints( hwnd, 0, lppnt, 1 );
+
+ if (GetLastError() != 0xdeadbeef)
+ return FALSE;
+
+ SetLastError(error);
return TRUE;
}
/*******************************************************************
- * ScreenToClient (USER32.@)
+ * ScreenToClient (USER32.@)
*/
BOOL WINAPI ScreenToClient( HWND hwnd, LPPOINT lppnt )
{
+ DWORD error = GetLastError();
+
+ if (!hwnd)
+ {
+ SetLastError( ERROR_INVALID_WINDOW_HANDLE );
+ return FALSE;
+ }
+
+ SetLastError( 0xdeadbeef );
MapWindowPoints( 0, hwnd, lppnt, 1 );
+
+ if (GetLastError() != 0xdeadbeef)
+ return FALSE;
+
+ SetLastError(error);
return TRUE;
}
@@ -422,7 +450,7 @@ HWND WINAPI ChildWindowFromPointEx( HWND hwndParent, POINT pt, UINT uFlags)
* Calculate the offset between the origin of the two windows. Used
* to implement MapWindowPoints.
*/
-static POINT WINPOS_GetWinOffset( HWND hwndFrom, HWND hwndTo, BOOL *mirrored )
+static BOOL WINPOS_GetWinOffset( HWND hwndFrom, HWND hwndTo, BOOL *mirrored, POINT *point )
{
WND * wndPtr;
POINT offset;
@@ -432,10 +460,20 @@ static POINT WINPOS_GetWinOffset( HWND hwndFrom, HWND hwndTo, BOOL *mirrored )
offset.x = offset.y = 0;
*mirrored = mirror_from = mirror_to = FALSE;
+ if ((hwndFrom && !IsWindow(hwndFrom)) || (hwndTo && !IsWindow(hwndTo)))
+ {
+ *point = offset;
+ return FALSE;
+ }
+
/* Translate source window origin to screen coords */
if (hwndFrom)
{
- if (!(wndPtr = WIN_GetPtr( hwndFrom ))) return offset;
+ if (!(wndPtr = WIN_GetPtr( hwndFrom )))
+ {
+ *point = offset;
+ return FALSE;
+ }
if (wndPtr == WND_OTHER_PROCESS) goto other_process;
if (wndPtr != WND_DESKTOP)
{
@@ -466,7 +504,11 @@ static POINT WINPOS_GetWinOffset( HWND hwndFrom, HWND hwndTo, BOOL *mirrored )
/* Translate origin to destination window coords */
if (hwndTo)
{
- if (!(wndPtr = WIN_GetPtr( hwndTo ))) return offset;
+ if (!(wndPtr = WIN_GetPtr( hwndTo )))
+ {
+ *point = offset;
+ return FALSE;
+ }
if (wndPtr == WND_OTHER_PROCESS) goto other_process;
if (wndPtr != WND_DESKTOP)
{
@@ -496,9 +538,10 @@ static POINT WINPOS_GetWinOffset( HWND hwndFrom, HWND hwndTo, BOOL *mirrored )
*mirrored = mirror_from ^ mirror_to;
if (mirror_from) offset.x = -offset.x;
- return offset;
+ *point = offset;
+ return TRUE;
- other_process: /* one of the parents may belong to another process, do it the hard way */
+other_process: /* one of the parents may belong to another process, do it the hard way */
offset.x = offset.y = 0;
SERVER_START_REQ( get_windows_offset )
{
@@ -512,19 +555,22 @@ static POINT WINPOS_GetWinOffset( HWND hwndFrom, HWND hwndTo, BOOL *mirrored )
}
}
SERVER_END_REQ;
- return offset;
+ *point = offset;
+ return TRUE;
}
/* map coordinates of a window region */
void map_window_region( HWND from, HWND to, HRGN hrgn )
{
BOOL mirrored;
- POINT offset = WINPOS_GetWinOffset( from, to, &mirrored );
+ POINT offset;
UINT i, size;
RGNDATA *data;
HRGN new_rgn;
RECT *rect;
+ WINPOS_GetWinOffset( from, to, &mirrored, &offset );
+
if (!mirrored)
{
OffsetRgn( hrgn, offset.x, offset.y );
@@ -552,14 +598,20 @@ void map_window_region( HWND from, HWND to, HRGN hrgn )
/*******************************************************************
- * MapWindowPoints (USER32.@)
+ * MapWindowPoints (USER32.@)
*/
INT WINAPI MapWindowPoints( HWND hwndFrom, HWND hwndTo, LPPOINT lppt, UINT count )
{
BOOL mirrored;
- POINT offset = WINPOS_GetWinOffset( hwndFrom, hwndTo, &mirrored );
+ POINT offset;
UINT i;
+ if (!WINPOS_GetWinOffset( hwndFrom, hwndTo, &mirrored, &offset ))
+ {
+ SetLastError( ERROR_INVALID_WINDOW_HANDLE );
+ return 0;
+ }
+
for (i = 0; i < count; i++)
{
lppt[i].x += offset.x;
More information about the wine-patches
mailing list