RE: Proposed solution for COMCTL32 subclassing problems

[Robert Shearman]

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

Proposed solution for COMCTL32 subclassing problems

[Filip Navara]

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

Proposed solution for COMCTL32 subclassing problems

[Robert Shearman]

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

Proposed solution for COMCTL32 subclassing problems

[Filip Navara]

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--;

Proposed solution for COMCTL32 subclassing problems

[Robert Shearman]

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