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

2017-03-08 Thread Francis Deslauriers
The previous implementation assumed that `atomic_add_unless` returned the
prior value of the atomic counter when in fact it returned if addition was
successful(true) or overflowed(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 
---
 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

___
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev


Re: [lttng-dev] [PATCH lttng-modules] Fix: atomic_add_unless() returns true/false rather than prior value

2017-03-08 Thread Mathieu Desnoyers
- On Mar 8, 2017, at 11:38 AM, Francis Deslauriers 
francis.deslauri...@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 addition was
> successful(true) or overflowed(false).

I think you mean "if addition was performed (true) or not performed (false)" ?

Thanks,

Mathieu

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

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
___
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev