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

[Rémi Denis-Courmont]

Le maanantaina 2. heinäkuuta 2018, 21.09.03 EEST Romain Vimont a écrit :
> On Mon, Jul 02, 2018 at 08:19:01PM +0300, Rémi Denis-Courmont wrote:
> > 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.
> 
> TBH, I would not be against removing these specific asserts, since I
> think they have low value (but maybe I am missing something).
> 
> Concretely, they test whether the refcount was positive before inc() or
> dec().

The asserts protect against "logical" use-after-free and overflow. The earlier 
has proven useful in the past.

You could argue that the bugs have been fixed long ago, and that sanitizers 
have superseded such assertions anyway. However I do not believe that all 
active developers actually use sanitizers routinely.

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