[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