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

[Mathieu Desnoyers]

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

Thanks,

Mathieu

> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> CC: Paul E. McKenney <paulmck at linux.vnet.ibm.com>
> CC: Lai Jiangshan <jiangshanlai at gmail.com>
> CC: Stephen Hemminger <stephen at networkplumber.org>
> CC: Chris Mason <clm at fb.com>
> CC: lttng-dev at lists.lttng.org
> CC: rp at svcs.cs.pdx.edu
> ---
> urcu/ref.h | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/urcu/ref.h b/urcu/ref.h
> index 2b803e5..42417c0 100644
> --- a/urcu/ref.h
> +++ b/urcu/ref.h
> @@ -34,19 +34,20 @@ static inline void urcu_ref_init(struct urcu_ref *ref)
> 	urcu_ref_set(ref, 1);
> }
> 
> -static inline void urcu_ref_get(struct urcu_ref *ref)
> +static inline bool  __attribute__((warn_unused_result))
> +		urcu_ref_get(struct urcu_ref *ref)
> {
> 	long old, _new, res;
> 
> 	old = uatomic_read(&ref->refcount);
> 	for (;;) {
> 		if (old == LONG_MAX) {
> -			abort();
> +			return false;	/* Failure. */
> 		}
> 		_new = old + 1;
> 		res = uatomic_cmpxchg(&ref->refcount, old, _new);
> 		if (res == old) {
> -			return;
> +			return true;	/* Success. */
> 		}
> 		old = res;
> 	}
> --
> 2.1.4

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