[vlc-devel] [PATCH v6] vlc_vector: add helpers for vectors

[Romain Vimont]

On Fri, Oct 05, 2018 at 09:32:38PM +0200, Romain Vimont wrote:
> Thank you for your review.
> 
> On Fri, Oct 05, 2018 at 08:20:09PM +0300, Rémi Denis-Courmont wrote:
> > Le perjantaina 5. lokakuuta 2018, 18.29.05 EEST Romain Vimont a écrit :
> > > +/**
> > > + * Clear a vector, and release any associated resources.
> > > + */
> > > +#define vlc_vector_clear(pv) \
> > > +( \
> > > +    /* cannot be implemened as do-while(0), called from vlc_vector_resize()
> > > */ \ +    free((pv)->data), \
> > > +    vlc_vector_init(pv) \
> > > +)
> > 
> > Bad naming. This does not actually clear anything.
> 
> Why do you say that it does not clear anything? It clears the vector.
> 
> Before the call, the vector has any number of items, after the call,
> it's empty.

Maybe the confusion was that _clear() was also used as _destroy().

Since the fact that _clear() frees all resources is an implementation
detail, I just added a new _destroy() macro:

    #define vlc_vector_destroy(pv) free((pv)->data)

> > > +/**
> > > + * Resize the vector to `newcap` exactly.
> > > + *
> > > + * If `newcap` is 0, the vector is cleared.
> > > + *
> > > + * \param pv a pointer to the vector
> > > + * \param newcap (size_t) the requested capacity
> > > + * \retval true if no allocation failed
> > > + * \retval false on allocation failure (the vector is left untouched)
> > > + */
> > > +#define vlc_vector_resize(pv, newcap) \
> > > +( \
> > > +    (pv)->cap == (newcap) /* nothing to do */ || \
> > > +    ( \
> > > +        (newcap) > 0 ? vlc_vector_realloc_(pv, newcap) \
> > > +                     : (vlc_vector_clear(pv), true) \
> > > +    ) \
> > > +)
> > 
> > Multiple expansion of newcap could cause problems?
> 
> In practice, probably not, but avoiding multiple expansion would be
> better.

I just made this macro private (by adding a '_'): vlc_vector_resize_().
I think we don't need to expose it, and that way there is no harmful
multi-expansion.