diff mbox

[lttng-modules,v3] Fix: atomic_add_unless() returns true/false rather than prior value

Message ID 1488991838-13807-1-git-send-email-francis.deslauriers@efficios.com
State Accepted, archived
Headers show

Commit Message

Francis Deslauriers March 8, 2017, 4:50 p.m. UTC
The previous implementation assumed that `atomic_add_unless` returned
the prior value of the atomic counter when in fact it returned if the
addition was performed (true) or not performed (false).
Since `atomic_add_unless` can not return INT_MAX, the `lttng_kref_get`
always returned that the call was successful.

This issue had a low likelihood of being triggered since the two refcounts
of the counters used with this call are both bounded by the maximum
number of file descriptors on the system.

Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
---
 wrapper/kref.h | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Mathieu Desnoyers March 8, 2017, 4:56 p.m. UTC | #1
merged into master, 2.9, 2.8, thanks!

Mathieu

----- On Mar 8, 2017, at 11:50 AM, Francis Deslauriers francis.deslauriers at efficios.com wrote:

> The previous implementation assumed that `atomic_add_unless` returned
> the prior value of the atomic counter when in fact it returned if the
> addition was performed (true) or not performed (false).
> Since `atomic_add_unless` can not return INT_MAX, the `lttng_kref_get`
> always returned that the call was successful.
> 
> This issue had a low likelihood of being triggered since the two refcounts
> of the counters used with this call are both bounded by the maximum
> number of file descriptors on the system.
> 
> Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
> ---
> wrapper/kref.h | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/wrapper/kref.h b/wrapper/kref.h
> index eedefbf..f30a9ae 100644
> --- a/wrapper/kref.h
> +++ b/wrapper/kref.h
> @@ -36,11 +36,7 @@
>  */
> static inline int lttng_kref_get(struct kref *kref)
> {
> -	if (atomic_add_unless(&kref->refcount, 1, INT_MAX) != INT_MAX) {
> -		return 1;
> -	} else {
> -		return 0;
> -	}
> +	return atomic_add_unless(&kref->refcount, 1, INT_MAX);
> }
> 
> #endif /* _LTTNG_WRAPPER_KREF_H */
> --
> 2.7.4
diff mbox

Patch

diff --git a/wrapper/kref.h b/wrapper/kref.h
index eedefbf..f30a9ae 100644
--- a/wrapper/kref.h
+++ b/wrapper/kref.h
@@ -36,11 +36,7 @@ 
  */
 static inline int lttng_kref_get(struct kref *kref)
 {
-	if (atomic_add_unless(&kref->refcount, 1, INT_MAX) != INT_MAX) {
-		return 1;
-	} else {
-		return 0;
-	}
+	return atomic_add_unless(&kref->refcount, 1, INT_MAX);
 }
 
 #endif /* _LTTNG_WRAPPER_KREF_H */