[vlc-devel] [PATCH] core: Add atomic refcounter helper

[Thomas Guillem]

On Sun, Jul 1, 2018, at 21:19, Rémi Denis-Courmont wrote:
> Le sunnuntaina 1. heinäkuuta 2018, 22.10.20 EEST Romain Vimont a écrit :
> > On Sun, Jul 01, 2018 at 09:48:03PM +0300, Rémi Denis-Courmont wrote:
> > > Le sunnuntaina 1. heinäkuuta 2018, 20.56.03 EEST Romain Vimont a écrit :
> > > > On Sun, Jul 01, 2018 at 08:10:54PM +0300, Rémi Denis-Courmont wrote:
> > > > > Le sunnuntaina 1. heinäkuuta 2018, 12.30.33 EEST Romain Vimont a écrit 
> :
> > > > > > On Sun, Jul 01, 2018 at 11:59:08AM +0300, Rémi Denis-Courmont wrote:
> > > > > > > Furthermore the boolean evaluation of the return value is the
> > > > > > > opposite
> > > > > > > of
> > > > > > > what any other ref counting API does (other than VLC's). Usually,
> > > > > > > zero
> > > > > > > is
> > > > > > > for last reference and non-zero for non-last references. Again,
> > > > > > > this
> > > > > > > is
> > > > > > > prone to errors.
> > > > > > 
> > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/t
> > > > > > ree/
> > > > > > lib/ refcount.c?h=v4.17#n208
> > > > > 
> > > > > This is very new.
> > > > 
> > > > In 2009, there were already atomic_dec_and_test(), with the same
> > > > behavior:
> > > > <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree
> > > > /inc lude/asm-generic/atomic.h?h=v2.6.31#n123>
> > > 
> > > So what?
> > 
> > So Linux have, at least since 2009, a function very similar to the
> > vlc_atomic_rc_dec() I propose, with the same behavior, and used in the
> > very same way. Here are random examples in 2009 (there are many others):
> > <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/inc
> > lude/net/ax25.h?h=v2.6.31#n171>
> > <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/in
> > clude/net/netlabel.h?h=v2.6.31#n252>
> > This contrasts with:
> > > the boolean evaluation of the return value is the opposite of what
> > > any other ref counting API does (other than VLC's). Usually, zero is
> > > for last reference and non-zero for non-last references.
> 
> Not really. Using a boolean is confusing because different APIs have it 
> different ways. By comparison, it is utterly obvious what atomic_fetch_OP 
> (value before op) or atomic_OP_return (value after op).

I really don't see how a boolean is confusing. We are following the same return value from the kernel, and if you have a doubt, just check the comment.

> 
> > and:
> > > This is very new.
> 
> Absolutely not. The refcount API in kernel is much newer than 2009. In fact, 
> it is so new that it is not even in AOSP yet. And it is largelly by security 
> people trying to remove a class of security issues that is only relevant to 
> the kernel.

Kernel AOSP is always late, I don't understand this argument.

This set is OK to me but I would split the API from the usage.

> 
> -- 
> レミ・デニ-クールモン
> http://www.remlab.net/
> 
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel