Question from new developer

Chris Teague chris.teague at gmail.com
Fri Sep 21 18:41:34 CDT 2012


I submitted bug 31753 for an application that I use, and started
making an attempt to fix it.  I started with the last few messages
from the log:

fixme:win:LockWindowUpdate (0x501c4), partial stub!
fixme:win:LockWindowUpdate ((nil)), partial stub!
wine: Unhandled page fault on read access to 0x00000023 at address
0x78680087 (thread 0009), starting debugger...

Looking at LockWindowUpdate(), I discovered that it had no conformance
tests.  I wrote a few covering all the possible scenarios I could
think of (only 4 of them), and ran them on wine as well as cross
compiling them and running on XP.  To my surprise, one of the tests
fails on wine, but passes on WinXP!  This was great news, so I dug a
little further.  Specifically the problem was attempting to do a
"unlock" operation by passing NULL, when the the function was already
unlocked.  Windows treats this as an error and returns 0, while wine
returns 1.
So I fixed it, patch is attached (also at:
https://dl.dropbox.com/u/477050/0001-Fixed-LockWindowUpdate.txt)
I have two big questions:
1. I discovered later that I don't think this is the root of my
original bug - and in fact doesn't seem to affect behavior of my
program.  Is it still worth submitting?  I do believe it alleviates
the need for the "fixme" in the LockWindowUpdate function.
2. If it is worth submitting, could someone take a look at it and
point out all the dumb things I've done?  I'm happy with the logic of
the code, but I'm sure I've done something horribly wrong in terms of
doing it the preferred way.

Thanks,
Chris Teague
-------------- next part --------------
From 84445fb85a4a22588af798195fcefd7db31b17fd Mon Sep 17 00:00:00 2001
From: root <root at kona-linux.qualcomm.com>
Date: Fri, 21 Sep 2012 23:24:43 +0000
Subject: Fixed LockWindowUpdate

---
 dlls/user32/painting.c        |   43 +++++++++--------
 dlls/user32/tests/Makefile.in |    1 +
 dlls/user32/tests/painting.c  |  103 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 128 insertions(+), 19 deletions(-)
 create mode 100644 dlls/user32/tests/painting.c

diff --git a/dlls/user32/painting.c b/dlls/user32/painting.c
index 1a6de87..c70298e 100644
--- a/dlls/user32/painting.c
+++ b/dlls/user32/painting.c
@@ -1154,32 +1154,37 @@ HWND WINAPI WindowFromDC( HDC hdc )
  */
 BOOL WINAPI LockWindowUpdate( HWND hwnd )
 {
-    static HWND lockedWnd;
+  static HWND lockedWnd;
 
-    FIXME("(%p), partial stub!\n",hwnd);
-
-    USER_Lock();
-    if (lockedWnd)
+  USER_Lock();
+  if (lockedWnd)
     {
-        if (!hwnd)
-        {
-            /* Unlock lockedWnd */
-            /* FIXME: Do something */
-        }
-        else
-        {
-            /* Attempted to lock a second window */
-            /* Return FALSE and do nothing */
-            USER_Unlock();
-            return FALSE;
+      if (hwnd)
+	{
+	  /* Attempted to lock a second window */
+	  /* Return FALSE and do nothing */
+	  USER_Unlock();
+	  return FALSE;
         }
     }
-    lockedWnd = hwnd;
-    USER_Unlock();
-    return TRUE;
+  else  /* Not locked */
+    {
+      if (!hwnd)
+	{
+	  /* Attempted to unlock while already unlocked */
+	  /* Return FALSE and do nothing */
+	  USER_Unlock();
+	  return FALSE;
+	}
+      
+    }
+  lockedWnd = hwnd;
+  USER_Unlock();
+  return TRUE;
 }
 
 
+
 /***********************************************************************
  *		RedrawWindow (USER32.@)
  */
diff --git a/dlls/user32/tests/Makefile.in b/dlls/user32/tests/Makefile.in
index eb5789a..24ff612 100644
--- a/dlls/user32/tests/Makefile.in
+++ b/dlls/user32/tests/Makefile.in
@@ -17,6 +17,7 @@ C_SRCS = \
 	menu.c \
 	monitor.c \
 	msg.c \
+	painting.c \
 	resource.c \
 	scroll.c \
 	static.c \
diff --git a/dlls/user32/tests/painting.c b/dlls/user32/tests/painting.c
new file mode 100644
index 0000000..8b08ea2
--- /dev/null
+++ b/dlls/user32/tests/painting.c
@@ -0,0 +1,103 @@
+#include <assert.h>
+#include <windows.h>
+#include <wine/test.h>
+#include <winbase.h>
+
+/* Maybe auxiliary functions and definitions here */
+static HINSTANCE hinst;
+static HWND create_editcontrol (DWORD style, DWORD exstyle)
+{
+  HWND handle;
+
+  handle = CreateWindowEx(exstyle,
+                          "EDIT",
+                          "Test Text",
+                          style,
+                          10, 10, 300, 300,
+                          NULL, NULL, hinst, NULL);
+  ok (handle != NULL, "CreateWindow EDIT Control failed\n");
+  assert (handle);
+  if (winetest_interactive)
+    ShowWindow (handle, SW_SHOW);
+  return handle;
+}
+
+/* lock valid window, nothing locked */
+static void test_lockwindowupdate_1(void)
+{  
+  HWND handle;
+  BOOL ret;
+
+  handle = create_editcontrol(ES_AUTOHSCROLL | ES_AUTOVSCROLL, 0);
+  assert (handle);
+
+  /* normal, successful window lock */
+  ret = LockWindowUpdate(handle);
+  ok(0 != ret, "LockWindowUpdate: scenario 1 failed to lock\n");
+
+  /* locking NULL when a window is locked will unlock it */
+  ret = LockWindowUpdate(NULL);
+  ok(0 != ret, "LockWindowUpdate: scenario 1 failed \n");
+}
+
+/* lock valid window, already locked */
+static void test_lockwindowupdate_2(void)
+{  
+  HWND handle;
+  HWND handle_2;
+  BOOL ret;
+
+  handle = create_editcontrol(ES_AUTOHSCROLL | ES_AUTOVSCROLL, 0);
+  assert (handle);
+  handle_2 = create_editcontrol(ES_AUTOHSCROLL | ES_AUTOVSCROLL, 0);
+  assert (handle_2);
+
+  /* normal, successful window lock */
+  ret = LockWindowUpdate(handle);
+  ok(0 != ret, "LockWindowUpdate: scenario 2 failed to lock\n");
+
+  /* attempt to lock a second window should fail */
+  ret = LockWindowUpdate(handle_2);
+  ok(0 == ret, "LockWindowUpdate: scenario 2 failed on second lock\n");
+
+  /* lock NULL to clear the lock */
+  ret = LockWindowUpdate(NULL);
+  ok(0 != ret, "LockWindowUpdate: scenario 2 failed to clear lock\n");
+}
+
+
+/* lock NULL window, nothing locked */
+static void test_lockwindowupdate_3(void)
+{  
+  BOOL ret;
+  /* locking NULL when nothing is locked should be an error */
+  ret = LockWindowUpdate(NULL);
+  ok(0 == ret, "LockWindowUpdate: scenario 3 failed, returned %d\n", ret);
+}
+
+/* lock NULL window, already locked */
+static void test_lockwindowupdate_4(void)
+{  
+  HWND handle;
+  BOOL ret;
+
+  handle = create_editcontrol(ES_AUTOHSCROLL | ES_AUTOVSCROLL, 0);
+  assert (handle);
+
+  /* normal, successful window lock */
+  ret = LockWindowUpdate(handle);
+  ok(0 != ret, "LockWindowUpdate: scenario 4 failed to lock window\n");
+
+  /* locking NULL should unlock the window */
+  ret = LockWindowUpdate(NULL);
+  ok(0 != ret, "LockWindowUpdate: scenario 4 failed to unlock window\n");
+}
+
+START_TEST(painting)
+{
+  trace("testing LockWindowUpdate\n");
+  test_lockwindowupdate_1();
+  test_lockwindowupdate_2();
+  test_lockwindowupdate_3();
+  test_lockwindowupdate_4();
+}
-- 
1.7.9.5


More information about the wine-devel mailing list