Filip Navara wrote:
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.
Yes, that was my fault. Sorry.
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.
It looks much simpler now, but I don't understand why reversing the direction of the stack should make any difference. I don't think you submitted the required patch to comctl32.h.
Rob
Robert Shearman wrote:
It looks much simpler now, but I don't understand why reversing the direction of the stack should make any difference.
Because when the DefSubclassProc is called first time the stackpos should be 0 and so if 0 is treated as topmost subclassed procedure in the hiearchy there's no longer needed SubclassWndProc. (Does it make sense? ;-) Unfortunetly my patch doesn't handle the stackpos position properly, so it wouldn't not work just yet.
I don't think you submitted the required patch to comctl32.h.
Argh, sorry. Nevertheless there is couple of bugs in my patch, so I'll send it again after fixing them.
- Filip
Filip Navara wrote:
Robert Shearman wrote:
It looks much simpler now, but I don't understand why reversing the direction of the stack should make any difference.
Because when the DefSubclassProc is called first time the stackpos should be 0 and so if 0 is treated as topmost subclassed procedure in the hiearchy there's no longer needed SubclassWndProc. (Does it make sense? ;-) Unfortunetly my patch doesn't handle the stackpos position properly, so it wouldn't not work just yet.
Can you not just use stack->stacknum instead of 0 if the stack is inverted? I'm not arguing against it and I'm all for it if it makes it easier to code, but I'm just trying to see what stops you from using the same stack direction as at the moment.
Rob
Robert Shearman wrote:
Can you not just use stack->stacknum instead of 0 if the stack is inverted?
Of course I can (now that I understand the code ;-).
I'm not arguing against it and I'm all for it if it makes it easier to code, but I'm just trying to see what stops you from using the same stack direction as at the moment.
If there is enough interest, I can recode it with using the previous stack layout. When I started the patch I just thought it would be easier this way, but now I can reasonably say that it wouldn't be much different.
- Filip
P.S. In the last patch I sent there was still one bug: + stack->stacknum--; + if (n < stack->stackpos) stack->stackpos--; should be + stack->stacknum--; + if (n < stack->stackpos && stack->wndprocrecursion) stack->stackpos--;
Filip Navara wrote:
Robert Shearman wrote:
Can you not just use stack->stacknum instead of 0 if the stack
is inverted?
Of course I can (now that I understand the code ;-).
I'm not arguing against it and I'm all for it if it makes it
easier to code,
but I'm just trying to see what stops you from using the same stack direction as at the moment.
If there is enough interest, I can recode it with using the previous stack layout. When I started the patch I just thought it would be easier this way, but now I can reasonably say that it wouldn't be much different.
Ok, that's fair enough. The patch looks good to me.
P.S. In the last patch I sent there was still one bug:
stack->stacknum--;
if (n < stack->stackpos) stack->stackpos--;
should be
stack->stacknum--;
if (n < stack->stackpos && stack->wndprocrecursion) stack->stackpos--;
Ok, I suggest you resubmit the patch with those changes if you haven't already done so.
Rob