[vlc-devel] [PATCH] core: Add atomic refcounter helper
remi at remlab.net
Sun Jul 1 10:59:08 CEST 2018
Le samedi 30 juin 2018, 23:49:23 EEST Romain Vimont a écrit :
> On Sat, Jun 30, 2018 at 10:28:14PM +0200, Rémi Denis-Courmont wrote:
> > The only times I recall that somebody screwed up reference counting where
> > nontrivial cases not handled by this patch.
> In network/httpd.c, httpd_HostDelete():
> if (atomic_fetch_sub_explicit(&host->ref, 1, memory_order_relaxed) > 1)
> I'm not sure a memory_order_relaxed is sufficient here, for the reasons
> explained in the video (the data need to be seen by the thread that
> destroys the object).
I fail to see what a video on reference counting would have to do with this.
> > And the Boolean result is not obvious at all. As far as error proneness
> > comes, this is actually *worse*.
> That's an opinion, I don't share it.
No it is not an opinion. It is a count. This patch contains two already
mentioned errors, three with the above paragraph. That is probably more than
the current approach has ever had in many years. Three is more than zero, and
that makes this patch more prone to errors based on circumstancial evidence.
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
And to top all of that, this does not even address the actually tricky cases
where programmers are actually prone to making non-obvious mistakes.
So no thanks.
More information about the vlc-devel