Re: problems compiling NSAnimation.m

[Xavier Glattard]

Richard Frith-Macdonald <richard <at> tiptree.demon.co.uk> writes:

> 
> 
> On 10 May 2007, at 14:57, Xavier Glattard wrote:
> 
> > Richard Frith-Macdonald <richard <at> tiptree.demon.co.uk> writes:
> >
(...)
> >
> > This macro is used as a macro : to repeat again and again the same
> > piece of code. It only has to be used in this class. If you have two
> > _NSANIMATION_LOCK then you have made an error : would you code two
> > [NSThread -lock] in sequence ?
> 
> You might ... if one method locks  then calls another method which  
> also locks.  Of course, if this is possible then the lock needs to be  
> a recursive one.

We are talking of two _NSANIMATION_LOCK in sequence _in_the_same_method_.
As it is present in (near) all methods IT IS called many time in sequence !

> >> I fixed this to make __gs_isLocked an ivar rather than declaring it
> >> locally (which was pretty suboptimal).  I also fixed a bug in the
> >> unlock macro (it was setting the lag to the wrong value), and added
> >> assertions to check that the macro is not misused.
> >
> > Bad. If __gs_locked is needed (and as i said i think it is not : i  
> > made
> > an error) then it must be declared in all methods. If __gs_isLocked is
> > an ivar, its value can be changed in another method then the [unlock]
> > will never be called.
> 
> Sure, but similarly if it's a local variable its value can be  
> incorrectly changed within a method or function where it is declared.

If it is then you made an other programming error... ;-)
Its name is prefixed by an underscore (and by 'gs'!) so it shouldn't
be used by anyone.
Moreover an ivar is more easier to be incorrectly changed...
in an other method or even in a child class!

> > An ivar already exists: _isThreaded.
> 
> But that doesn't record the same thing.  The __gs_locked boolean  
> records whether the lock is locked, not whether the animation is  
> threaded.

I agree : _gs_isLocked records whether the _local_ lock is locked.
So it has to be a _local_ variable.
_gs_isLocked as an ivar is an error.

> (...) I only looked at the code enough to  
> fix it to compile and manage locking correctly
> (...)

Did you run GSTest ?

Regards

Xavier