[vlc-devel] [PATCH 2/3] queue: helpers for killable queues

[Rémi Denis-Courmont]

Le tiistaina 14. huhtikuuta 2020, 8.43.24 EEST Steve Lhomme a écrit :
> On 2020-04-11 21:12, Rémi Denis-Courmont wrote:
> > This adds two functions to simplify the common case of a thread waiting on
> > a queue until it is terminated.
> > ---
> > 
> >   include/vlc_queue.h | 42 ++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 42 insertions(+)
> > 
> > diff --git a/include/vlc_queue.h b/include/vlc_queue.h
> > index eed69510dc..f41d72fb55 100644
> > --- a/include/vlc_queue.h
> > +++ b/include/vlc_queue.h
> > @@ -214,5 +214,47 @@ VLC_USED;
> > 
> >    */
> >   
> >   VLC_API void *vlc_queue_DequeueAll(vlc_queue_t *) VLC_USED;
> > 
> > +/**
> > + * @defgroup queue_killable Killable queues
> > + *
> > + * Thread-safe queues with an end flag.
> > + *
> > + * @{
> > + */
> > +
> > +/**
> > + * Marks a queue ended.
> > + */
> > +static inline void vlc_queue_Kill(vlc_queue_t *q,
> > +                                  bool *restrict tombstone)
> > +{
> > +    vlc_queue_Lock(q);
> > +    *tombstone = true;
> 
> What's the point of returning a bool if it's always the same value. And
> why not return it directly rather than using a pointer ?

There would be no point returning a constant indeed. And the code doesn't.

> > +    vlc_queue_Signal(q);
> > +    vlc_queue_Unlock(q);
> > +}
> > +
> > +/**
> > + * Dequeues one entry from a killable queue.
> > + *
> > + * @return an entry, or NULL if the queue is empty and has been ended.
> > + */
> > +static inline void *vlc_queue_DequeueKillable(vlc_queue_t *q,
> > +                                              ptrdiff_t offset,
> > +                                              bool *restrict tombstone)
> > +{
> > +    void *entry;
> > +
> > +    vlc_queue_Lock(q);
> > +    while (vlc_queue_IsEmpty(q) && !*tombstone)
> 
> I would put the bool test in a surrounding if().

That's not how condition variables work.

> > +        vlc_queue_Wait(q);
> > +
> > +    entry = vlc_queue_DequeueUnlocked(q, offset);
> > +    vlc_queue_Unlock(q);
> > +    return entry;
> > +}
> > +
> > +/** @} */
> > +
> > 
> >   /** @} */
> >   #endif
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


-- 
Rémi Denis-Courmont
Tapiola new town, Uusimaan Republic