On Wed, Oct 28, 2015 at 06:01:02PM +0300, Denis V. Lunev wrote: > +int rfifolock_is_locked(RFifoLock *r);
Please use bool instead of int. > diff --git a/util/rfifolock.c b/util/rfifolock.c > index afbf748..8ac58cb 100644 > --- a/util/rfifolock.c > +++ b/util/rfifolock.c > @@ -48,7 +48,7 @@ void rfifolock_lock(RFifoLock *r) > /* Take a ticket */ > unsigned int ticket = r->tail++; > > - if (r->nesting > 0 && qemu_thread_is_self(&r->owner_thread)) { > + if (r->nesting > 0 && rfifolock_is_locked(r)) { > r->tail--; /* put ticket back, we're nesting */ > } else { > while (ticket != r->head) { > @@ -69,10 +69,15 @@ void rfifolock_unlock(RFifoLock *r) > { > qemu_mutex_lock(&r->lock); > assert(r->nesting > 0); > - assert(qemu_thread_is_self(&r->owner_thread)); > + assert(rfifolock_is_locked(r)); > if (--r->nesting == 0) { > r->head++; > qemu_cond_broadcast(&r->cond); > } > qemu_mutex_unlock(&r->lock); > } > + > +int rfifolock_is_locked(RFifoLock *r) > +{ > + return qemu_thread_is_self(&r->owner_thread); > +} The function name confused me since "does the current thread hold the lock?" != "does anyone currently hold the lock?". I suggest: bool rfifolock_held_by_current_thread(RFifoLock *r) { return r->nesting > 0 && qemu_thread_is_self(&r->owner_thread); } Then the r->nesting > 0 testing can also be dropped by callers, which is good since rfifolock_is_locked() does not return a meaningful result when r->nesting == 0.
signature.asc
Description: PGP signature