Re: setNextKeyView: dangling pointers

[Richard Frith-Macdonald]

On Monday, October 21, 2002, at 12:59  am, Nicola Pero wrote:

> > The doubly linked list that keeps track of the next/previous key views
> > in NSView can be messed up easily, leaving behind dangling pointers
> > to deallocated views.
> > (I was using a CVS version from october 19th).
>
> > Here is a test program:
>
> Thanks - brilliant
>
>
> > As you can see, the list structure is inconsistent, resulting in a 
> > message
> > sent to the deallocated view c.
>
> Yes :-)
>
>
> > I suggest to rewrite the setNextKeyView/setPreviousKeyView methods
> > in a way that they never produce inconsistent lists.
>
> Agreed!
>
>
> > Here is a patch to NSView.m
> > The code is a little bit tricky, it tries to assign the right
> > values to the right fields at the right time, so that the recursive
> > message sends finally terminate.
>
> Thanks! - yes I made similar changes, with minor differences ...
>
>
> > 2002-10-20 16:43:11.575 MyTest[17896] setNextKeyView of a to c
> > 2002-10-20 16:43:11.590 MyTest[17896] setPreviousKeyView of c to a
> > 2002-10-20 16:43:11.590 MyTest[17896] setNextKeyView of b to c
> > 2002-10-20 16:43:11.591 MyTest[17896] setPreviousKeyView of c to b
> > 2002-10-20 16:43:11.591 MyTest[17896] setNextKeyView of a to 0
> > 2002-10-20 16:43:11.592 MyTest[17896] setPreviousKeyView of c to 0
>
> ... in particular, I wanted to prevent the code from doing this
> `setPreviousKeyView of c to 0' (which is sort of an internally 
> generated
> call), so that we only and exactly perform the calls to setup the key 
> view
> chain.
>
> I know it doesn't make much of a difference in most practical cases :-)
> but I thought if you subclass NSView and override setPreviousKeyView: 
> to
> do something more than it does in NSView, you don't want it to be 
> called
> spuriously - it's better if we call each set{Next,Previous}KeyView: 
> method
> only once for each involved view, to set up the exact final 
> next/previous
> view, avoiding 'internal/bogus' calls.
>
> Thanks for contributing - let me know if it still doesn't work! :-)


Well ... seems we both looked at this at the weekend, and the stuff I 
did on it
was too time consuming to finish until this morning.

I've done a complete rewrite ... so I may have broken something (hope 
not).

Basically, the problem I found was this -
The MacOS-X implementation is actually quite different to what the 
MacOS-X
documentation implies.  The GNUstep implementation was also slightly
different (but in other ways).

A relatively minor issue is that MacOS-X has no -setPreviousKeyView: 
method ...
which means that all changes to the key-view chain are done by the 
-setNextKeyView:
method (except for some stuff done in -dealloc).

More importantly, the documentation talks about a key view loop ... but 
neither
MacOS-X nor GNUstep implement any such thing.

GNUstep implemented a simple doubly linked list terminated with nil at 
either
end, which *could* be made into a loop by joining the ends.

MacOS-X implemented a directed graph of one-to-many/many-to-one 
relationships,
which *could* be made into a loop as a special case.

Basically, what I did was try to mimic the MacOS-X behavior as closely 
as I could.

I'm not proud of the mechanism I chose to do this ... I'd like to 
rewrite it more
comprehensibly if/when I have time.  I chose a pair of arrays, with 
their zeroth
elements treated specially ... because I could do that without changing 
the ivar
layout in NSView.  I think it would be better as next/previous pointers 
for the
links and NSHashTables for keeping track of the references.