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

[Mathieu Desnoyers]

----- 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.

Thoughts ?

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com