Proposed solution for COMCTL32 subclassing problems

Filip Navara xnavara at volny.cz
Sat Apr 17 01:48:39 CDT 2004


Hi,

here is my proposed solution for crashes in COMCTL32 observed in 
specific situations while calling the window procedure subclassing 
functions. If there were removed all subclassed window procedures during 
the execution of SubclassWndProc there happened a crash, because the 
structure that holds the subclassing stack was freed in DefSubclassProc. 
Also if there were some window procedure subclassings removed during the 
execution of SubclassWndProc, the stack position counting could have 
gone wrong. My solution is to let the subclassing stack grow the 
opposite way, so that item 0 is the most nested one. I also removed the 
stacknew handling which I strongly believe is redundant now.

- Filip

-------------- next part --------------
--- comctl32/commctrl.c	Thu Mar 11 22:38:22 2004
+++ comctl32/commctrl.c	Sat Apr 17 06:29:42 2004
@@ -114,7 +114,6 @@
 extern void UPDOWN_Register(void);
 extern void UPDOWN_Unregister(void);
 
-static LRESULT WINAPI SubclassWndProc (HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam);
 
 LPSTR    COMCTL32_aSubclass = NULL;
 HMODULE COMCTL32_hModule = 0;
@@ -1109,10 +1108,10 @@
       /* set window procedure to our own and save the current one */
       if (IsWindowUnicode (hWnd))
          stack->origproc = (WNDPROC)SetWindowLongW (hWnd, GWL_WNDPROC,
-                                                   (LONG)SubclassWndProc);
+                                                   (LONG)DefSubclassProc);
       else
          stack->origproc = (WNDPROC)SetWindowLongA (hWnd, GWL_WNDPROC,
-                                                   (LONG)SubclassWndProc);
+                                                   (LONG)DefSubclassProc);
    } else {
       WNDPROC current;
       if (IsWindowUnicode (hWnd))
@@ -1120,7 +1119,7 @@
       else
          current = (WNDPROC)GetWindowLongA (hWnd, GWL_WNDPROC);
 
-      if (current != SubclassWndProc) {
+      if (current != DefSubclassProc) {
          ERR ("Application has subclassed with our procedure, then manually, then with us again.  The current implementation can't handle this.\n");
          return FALSE;
       }
@@ -1128,32 +1127,27 @@
 
    /* Check to see if we have called this function with the same uIDSubClass
     * and pfnSubclass */
-   for (n = 0; n <= stack->stacknum + stack->stacknew - 1; n++)
+   for (n = 0; n < stack->stacknum; n++)
       if ((stack->SubclassProcs[n].id == uIDSubclass) && 
          (stack->SubclassProcs[n].subproc == pfnSubclass)) {
          stack->SubclassProcs[n].ref = dwRef;
          return TRUE;
       }
 
-   if ((stack->stacknum + stack->stacknew) >= 32) {
+   if (stack->stacknum >= 32) {
       ERR ("We have a Subclass stack overflow, please increment size\n");
       return FALSE;
    }
 
-   /* we can't simply increment both stackpos and stacknum because there might
-    * be a window procedure running lower in the stack, we can only get them
-    * up to date once the last window procedure has run */
-   if (stack->stacknum == stack->stackpos) {
+   memmove (&stack->SubclassProcs[1], &stack->SubclassProcs[0],
+            sizeof(stack->SubclassProcs[0]) * stack->stacknum);
+
       stack->stacknum++;
       stack->stackpos++;
-   } else
-      stack->stacknew++;
-
-   newnum = stack->stacknew + stack->stacknum - 1;
 
-   stack->SubclassProcs[newnum].subproc = pfnSubclass;
-   stack->SubclassProcs[newnum].ref = dwRef;
-   stack->SubclassProcs[newnum].id = uIDSubclass;
+   stack->SubclassProcs[0].subproc = pfnSubclass;
+   stack->SubclassProcs[0].ref = dwRef;
+   stack->SubclassProcs[0].id = uIDSubclass;
    
    return TRUE;
 }
@@ -1188,7 +1182,7 @@
    if (!stack)
       return FALSE;
 
-   for (n = 0; n <= stack->stacknum + stack->stacknew - 1; n++)
+   for (n = 0; n < stack->stacknum; n++)
       if ((stack->SubclassProcs[n].id == uID) &&
          (stack->SubclassProcs[n].subproc == pfnSubclass)) {
          *pdwRef = stack->SubclassProcs[n].ref;
@@ -1226,7 +1220,8 @@
    if (!stack)
       return FALSE;
 
-   if ((stack->stacknum == 1) && (stack->stackpos == 1) && !stack->stacknew) {
+   if ((stack->stacknum == 1) && (stack->stackpos == 1) &&
+       !stack->wndprocrecursion) {
       TRACE("Last Subclass removed, cleaning up\n");
       /* clean up our heap and reset the origional window procedure */
       if (IsWindowUnicode (hWnd))
@@ -1238,24 +1233,21 @@
       return TRUE;
    }
  
-   for (n = stack->stacknum + stack->stacknew - 1; n >= 0; n--)
+   for (n = stack->stacknum - 1; n >= 0; n--)
       if ((stack->SubclassProcs[n].id == uID) &&
          (stack->SubclassProcs[n].subproc == pfnSubclass)) {
-         if (n != (stack->stacknum + stack->stacknew))
+         if (n != stack->stacknum)
             /* Fill the hole in the stack */
             memmove (&stack->SubclassProcs[n], &stack->SubclassProcs[n + 1],
-                    sizeof(stack->SubclassProcs[0]) * (stack->stacknew + stack->stacknum - n));
+                    sizeof(stack->SubclassProcs[0]) * (stack->stacknum - n));
          stack->SubclassProcs[n].subproc = NULL;
          stack->SubclassProcs[n].ref = 0;
          stack->SubclassProcs[n].id = 0;
 
-         /* If we are currently running a window procedure we have to manipulate
-          * the stack position pointers so that we don't corrupt the stack */
-         if ((n < stack->stackpos) || (stack->stackpos == stack->stacknum)) {
             stack->stacknum--;
+         if (n < stack->stackpos)
             stack->stackpos--;
-         } else if (n >= stack->stackpos)
-            stack->stacknew--;
+
          return TRUE;
       }
 
@@ -1264,33 +1256,6 @@
 
 
 /***********************************************************************
- * SubclassWndProc (internal)
- *
- * Window procedure for all subclassed windows. 
- * Saves the current subclassing stack position to support nested messages
- */
-
-static LRESULT WINAPI SubclassWndProc (HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
-{
-   LPSUBCLASS_INFO stack;
-   int stackpos;
-   LRESULT ret;
-
-   /* retrieve our little stack from the Properties */
-   stack = (LPSUBCLASS_INFO)GetPropA (hWnd, COMCTL32_aSubclass);
-   if (!stack) {
-      ERR ("Our sub classing stack got erased for %p!! Nothing we can do\n", hWnd);
-      return 0;
-   }
-   stackpos = stack->stackpos;
-   stack->stackpos = stack->stacknum;
-   ret = DefSubclassProc(hWnd,uMsg,wParam,lParam);
-   stack->stackpos = stackpos;
-   return ret;
-}
-
-
-/***********************************************************************
  * DefSubclassProc [COMCTL32.413]
  *
  * Calls the next window procedure (ie. the one before this subclass)
@@ -1319,25 +1284,24 @@
       return 0;
    }
 
-   /* If we are at pos 0 then we have to call the origional window procedure */
-   if (stack->stackpos == 0) {
+   stack->wndprocrecursion++;
+
+   /* If we are at the end of stack then we have to call the original
+    * window procedure */
+   if (stack->stackpos == stack->stacknum) {
       if (IsWindowUnicode (hWnd))
          return CallWindowProcW (stack->origproc, hWnd, uMsg, wParam, lParam);
       else
          return CallWindowProcA (stack->origproc, hWnd, uMsg, wParam, lParam);
    }
 
-   stackpos = --stack->stackpos;
-   /* call the Subclass procedure from the stack */
-   ret = stack->SubclassProcs[stackpos].subproc (hWnd, uMsg, wParam, lParam,
-         stack->SubclassProcs[stackpos].id, stack->SubclassProcs[stackpos].ref);
    stack->stackpos++;
+   /* call the Subclass procedure from the stack */
+   ret = stack->SubclassProcs[stack->stackpos - 1].subproc (hWnd, uMsg, wParam, lParam,
+         stack->SubclassProcs[stack->stackpos - 1].id, stack->SubclassProcs[stack->stackpos - 1].ref);
+   stack->stackpos--;
 
-   if ((stack->stackpos == stack->stacknum) && stack->stacknew) {
-      stack->stacknum += stack->stacknew;
-      stack->stackpos += stack->stacknew;
-      stack->stacknew = 0;
-   }
+   stack->wndprocrecursion--;
 
    /* If we removed the last entry in our stack while a window procedure was
     * running then we have to clean up */


More information about the wine-patches mailing list