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

[Rémi Denis-Courmont]

A Boolean is confusing because it is not intrinsically obvious which way it goes. But that is not really the main problem.

Le 2 juillet 2018 17:43:12 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>
>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
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180702/944c7994/attachment.html>