[vlc-devel] [PATCH v2 01/10] core: add atomic refcounter helper

Rémi Denis-Courmont remi at remlab.net
Mon Jul 2 19:19:01 CEST 2018


Le maanantaina 2. heinäkuuta 2018, 19.55.12 EEST Romain Vimont a écrit :
> On Mon, Jul 02, 2018 at 06:56:57PM +0300, Rémi Denis-Courmont wrote:
> > I prefer thread fence over release order but I still don't know any way to
> > make a helper without removing asserts, and I don't want to remove
> > asserts.
> Either:
>  0. we remove these refcount asserts (you don't want that);

*Nobody* wants to remove asserts, AFAIK.

>  1. we keep assert() in the header (like in other existing headers);

An assert() in a public header is a bug, unless within a macro (as in 
<vlc_common.h>). Been there done that. I _will_ remove any assert in public 
headers before 4.0 is out because this will break external plugins again 
otherwise.

People who added them better fix their code if they don't want that to happen.

>  2. we move the implementation to a .c and keep the asserts there;

That defeats the whole point of optimizing the memory barriers.

>  3. we implement the function as macro, so the clients need to include
>    assert.h themselves (thx robux4).

It does not work AFAICT. You can wrap the increment in do-while, because it 
has no value. But you cannot wrap the decrement.

You are assuming that helpers are necessary at all. Helpers are nice when they 
work nicely or when they factor large bits of code.

-- 
Rémi Denis-Courmont
http://www.remlab.net/





More information about the vlc-devel mailing list