diff mbox series

atomic.h: Reword confusing comment for qatomic_cmpxchg

Message ID 20240223182035.1048541-1-peter.maydell@linaro.org
State Superseded
Headers show
Series atomic.h: Reword confusing comment for qatomic_cmpxchg | expand

Commit Message

Peter Maydell Feb. 23, 2024, 6:20 p.m. UTC
The qatomic_cmpxchg() and qatomic_cmpxchg__nocheck() macros have
a comment that reads:
 Returns the eventual value, failed or not

This is somewhere between cryptic and wrong, since the value actually
returned is the value that was in memory before the cmpxchg.  Reword
to match how we describe these macros in atomics.rst.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu/atomic.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Richard Henderson Feb. 23, 2024, 6:21 p.m. UTC | #1
On 2/23/24 08:20, Peter Maydell wrote:
> The qatomic_cmpxchg() and qatomic_cmpxchg__nocheck() macros have
> a comment that reads:
>   Returns the eventual value, failed or not
> 
> This is somewhere between cryptic and wrong, since the value actually
> returned is the value that was in memory before the cmpxchg.  Reword
> to match how we describe these macros in atomics.rst.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   include/qemu/atomic.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
> index f1d3d1702a9..99110abefb3 100644
> --- a/include/qemu/atomic.h
> +++ b/include/qemu/atomic.h
> @@ -202,7 +202,7 @@
>       qatomic_xchg__nocheck(ptr, i);                          \
>   })
>   
> -/* Returns the eventual value, failed or not */
> +/* Returns the old value of '*ptr' (whether the cmpxchg failed or not) */
>   #define qatomic_cmpxchg__nocheck(ptr, old, new)    ({                   \
>       typeof_strip_qual(*ptr) _old = (old);                               \
>       (void)__atomic_compare_exchange_n(ptr, &_old, new, false,           \

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Jonathan Cameron Feb. 23, 2024, 6:23 p.m. UTC | #2
On Fri, 23 Feb 2024 08:21:42 -1000
Richard Henderson <richard.henderson@linaro.org> wrote:

> On 2/23/24 08:20, Peter Maydell wrote:
> > The qatomic_cmpxchg() and qatomic_cmpxchg__nocheck() macros have
> > a comment that reads:
> >   Returns the eventual value, failed or not
> > 
> > This is somewhere between cryptic and wrong, since the value actually
> > returned is the value that was in memory before the cmpxchg.  Reword
> > to match how we describe these macros in atomics.rst.
> > 
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   include/qemu/atomic.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
> > index f1d3d1702a9..99110abefb3 100644
> > --- a/include/qemu/atomic.h
> > +++ b/include/qemu/atomic.h
> > @@ -202,7 +202,7 @@
> >       qatomic_xchg__nocheck(ptr, i);                          \
> >   })
> >   
> > -/* Returns the eventual value, failed or not */
> > +/* Returns the old value of '*ptr' (whether the cmpxchg failed or not) */
> >   #define qatomic_cmpxchg__nocheck(ptr, old, new)    ({                   \
> >       typeof_strip_qual(*ptr) _old = (old);                               \
> >       (void)__atomic_compare_exchange_n(ptr, &_old, new, false,           \  
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

As the person it confused ;)
> 
> r~
Zhao Liu Feb. 26, 2024, 8:46 a.m. UTC | #3
On Fri, Feb 23, 2024 at 06:20:35PM +0000, Peter Maydell wrote:
> Date: Fri, 23 Feb 2024 18:20:35 +0000
> From: Peter Maydell <peter.maydell@linaro.org>
> Subject: [PATCH] atomic.h: Reword confusing comment for qatomic_cmpxchg
> X-Mailer: git-send-email 2.34.1
> 
> The qatomic_cmpxchg() and qatomic_cmpxchg__nocheck() macros have
> a comment that reads:
>  Returns the eventual value, failed or not
> 
> This is somewhere between cryptic and wrong, since the value actually
> returned is the value that was in memory before the cmpxchg.  Reword
> to match how we describe these macros in atomics.rst.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/qemu/atomic.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

> 
> diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
> index f1d3d1702a9..99110abefb3 100644
> --- a/include/qemu/atomic.h
> +++ b/include/qemu/atomic.h
> @@ -202,7 +202,7 @@
>      qatomic_xchg__nocheck(ptr, i);                          \
>  })
>  
> -/* Returns the eventual value, failed or not */
> +/* Returns the old value of '*ptr' (whether the cmpxchg failed or not) */
>  #define qatomic_cmpxchg__nocheck(ptr, old, new)    ({                   \
>      typeof_strip_qual(*ptr) _old = (old);                               \
>      (void)__atomic_compare_exchange_n(ptr, &_old, new, false,           \
> -- 
> 2.34.1
> 
>
diff mbox series

Patch

diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index f1d3d1702a9..99110abefb3 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -202,7 +202,7 @@ 
     qatomic_xchg__nocheck(ptr, i);                          \
 })
 
-/* Returns the eventual value, failed or not */
+/* Returns the old value of '*ptr' (whether the cmpxchg failed or not) */
 #define qatomic_cmpxchg__nocheck(ptr, old, new)    ({                   \
     typeof_strip_qual(*ptr) _old = (old);                               \
     (void)__atomic_compare_exchange_n(ptr, &_old, new, false,           \