[lttng-dev] [rp] [RFC PATCH urcu] urcu_ref_get: API change: return boolean

[Paul E. McKenney]

On Thu, Jan 21, 2016 at 05:06:09PM +0000, Mathieu Desnoyers wrote:
> ----- On Jan 21, 2016, at 11:59 AM, Josh Triplett josh at joshtriplett.org wrote:
> 
> > On Thu, Jan 21, 2016 at 04:45:20PM +0000, Mathieu Desnoyers wrote:
> >> ----- On Jan 19, 2016, at 3:57 PM, Mathieu Desnoyers
> >> mathieu.desnoyers at efficios.com wrote:
> >> 
> >> > This is a RFC of a follow up patch based on urcu commit 7d7c5d467 "Fix:
> >> > handle reference count overflow".
> >> > 
> >> > Change the urcu_ref_get prototype to return a boolean, which takes the
> >> > value false if a LONG_MAX overflow would occur (get has not been
> >> > performed), or true otherwise.
> >> > 
> >> > This interface change also introduces a "warn_unused_result" gcc
> >> > function attribute, which will show warnings if users don't handle the
> >> > return value.
> >> > 
> >> > I'm wondering whether this change is useful enough to justify breaking
> >> > the API (need to bump the major library version), or if introducing a
> >> > new "urcu_ref_get_safe()" or such would be a better option ?
> >> 
> >> After some thinking, I will go for adding a new urcu_ref_get_safe() API,
> >> thus not requiring to bump the library major version.
> > 
> > You may want to add a deprecated __attribute__ to the unsafe version in
> > the header.
> 
> The "unsafe" version now does a "abort()" in case of detected overflow,
> so it's not a security concern per-se, but could theoretically lead to
> an application denial of service.
> 
> I'm tempted to keep urcu_ref_get() as it is (not deprecate it) because
> there appears to be valid use-cases for it: for instance, if an application
> fully controls the reference counting (refcount value don't depend on
> external inputs), urcu_ref_get would still seem like a good API to use.

This seems to me to be a good tradeoff.  If experience shows that
urcu_ref_get() is "unsafe at any speed", then Josh's suggestion of
deprecating it would make a lot of sense.

							Thanx, Paul