[kaffe] Possibly Bug In the implementation of jcondvar_broadcast

Timothy Stack stack at cs.utah.edu
Thu Jun 3 13:34:01 PDT 2004


> 
> dai shaowei wrote:
> > Dear Jim Pick:
> >     I am currently analysing the source code of Kaffe,I find there are 
> > possibly something wrong in the implemention of the function  
> > "jcondvar_broadcast"and "jthread_stop"  in
> > kaffe -1.1.4\kaffe\kaffevm\systems\unix-jhtreads,following is the source 
> > code
> > 
> > I think the correct implementation should be :
> > Your Orginal code: for (condp = cv; *condp != 0; condp = &(*condp)->next)
> >             ;
> >                  (*condp) = lock->waiting;
> >                   lock->waiting = *condp;
> > My Corrected code: for (condp = cv; (*condp)->next != 0; condp = 
> > &(*condp)->next)
> >                   ;
> >                  (*condp) ->next = lock->waiting;
> >                   lock->waiting = *cv; after the "for" repeating 
> > clause,*condp == 0,this cause the node in the cv can't link the lock 
> > waiting queue,and after the link ,we should set the lock->waiting to be 
> > the first element of the new queue,am I right? hoping your reply.
> > 
> 
> Hi !
> 
> I agree there is a bug here. We are basically doing lock->waiting =
> lock->waiting which is wrong. However I believe that the loop is right
> (though your code may also work). When the loop exits, condp points to
> the last (*condp)->next variable (which is NULL by the loop condition.
> So we can assign it to lock->waiting by dereferencing condp. However
> lock->waiting should be assigned to the first node of the queue, that is
> *cv.
> 
> I think that when I modified that code I have made an extra copy-paste. ;-)

Frankly, this bit of code is just too confusing as it is written.  I, for
one, could never make heads or tails of it and it should just be rewritten 
in a clearer form.

> Guilhem Lavaux.

tim




More information about the kaffe mailing list