1. Michael Geary says:

   November 10, 2003 at 4:30 am

   Someone else can subclass the window while you have it subclassed. When you restore the original window function, you remove their subclass as well as your own.

   Window hooks don’t have this problem, because Windows manages the hook chain.

2. Anonymous says:

   November 10, 2003 at 4:54 am

   Doh. Didn’t think of that.

3. Centaur says:

   November 10, 2003 at 6:04 am

   First of all, I recall the blessed DOS days, where you added functionality to the operating system by writing memory-resident programs and hooked interrupt vectors. And sooner or later someone would ask for a way to uninstall your TSR from memory, and you would free the memory and restore old interrupt vectors as they were essentially pointers into your code, and if you didn’t restore them, they would dangle, resulting Undefined Behavior. And if you set the vectors to their old values, all was well. Until someone tried installing your TSR, installing some J. Random Programmer’s TSR, and then removing your TSR. Suddenly, the other TSR would stop working, because, instead of deleting yourself from a chain (a singly-linked list, to be precise), you bit off a whole section of the chain. Several arcane mechanisms were invented so that programs could unload themselves in any order the user pleased, but the majority of TSRs just kept that pointer to the previous in chain. They just compared the current vector pointer to a pointer to themselves, and if not equal, they gave the user “So don’t do that”.

   In short, the code shown above does not check if a window was subclassed by someone else on top of you. (That someone else would have to live in the same process as you and the window being subclassed, but you asked what *could* go wrong, and this certainly can.)

   And if you live in another process than hwnd, then you would have to jump through several hoops of different sizes so as to inject your code into hwnd’s process’s address space, and then again guard for resubclassing.

   And SubclassWindow is an undocumented macro which expands into a call to SetWindowLongPtr, which might mean that the name is deprecated. For the sake of code clarity, I think SetWindowLongPtr should have been used explicitly.

   Speaking of which, if we peek into the documentation on SetWindowLongPtr, we will see that upon calling this function we may also want to call SetWindowPos as it flushes the cache which caches some window data.

   And by the way, speaking of going wrong, usually it is not a question what *can* go wrong but how do we *know* that something went wrong. And usually we know that something went wrong when an API function returns zero. But zero is a valid return value for SetWindowLongPtr (although maybe not for GWLP_WNDPROC), and for extra protection we might probably want to explicitly implement the error checking procedure prescribed in the documentation:

   if (!SetLastError(0)) /* sh!t happened */;
   WNDPROC OldWndProc = SetWindowLongPtr(hwnd, GWL_WNDPROC, NewWndProc);
   if (NULL == OldWndProc)
   {
   DWORD lastError = GetLastError();
   if (0 != lastError)
   {
   /* sh!t happened */
   }
   }

   And, of course, NewWndProc must be ready to work with hwnd, and CallWindowProc(OldWndProc, …) for everything it doesn’t know how to handle.

   And if the hwnd’s message loop is in another thread, then it seems we have a race condition here, because SetWindowLongPtr will change the window proc for hwnd, then, before the return value of SetWindowLongPtr is copied into OldWndProc, the NewWndProc will get called, and will in turn call the thing pointed to by OldWndProc, which is not yet what it was supposed to call. So probably it would be safer to first GetWindowLongPtr(hwnd, GWLP_WNDPROC) into OldWndProc, and only then SetWindowLongPtr(hwnd, GWLP_WNDPROC, NewWndProc).

   Am I being too paranoid? :)

4. Ifeanyi Echeruo says:

   November 10, 2003 at 3:14 pm

   Eye opener. GJ.
   <MockEpiphany>So *that’s* why bug free software is SOOO hard to write the first time round</MockEpiphany>
   Point this article to the next person that says ‘testing shmesting we dont need none of that’

5. Ian Hanschen says:

   November 10, 2003 at 9:46 pm

   Specifically I’m sure you’re going to bring up the subclass chain and how one subclass on top of another with the removal of the other subclass could result in both subclasses in the chain being removed.

   The usual fix when in other code is just to say if(GetWindowLong(hWnd, GWL_WNDPROC) != pfnMySubclassProc).. Don’t drop the subclass, just the functionality inside of your wndproc, and callwindowproc[a/w] the wndproc previously returned.

   There are ways to maintain the subclass chain such that you can remove your subclass when another subclass has been made after it, but that’s not something I’m able to discuss.

6. The Old New Thing says:

   August 10, 2006 at 10:00 am

   Unhooking by hooking again.

Comments are closed.