The macro parameter 'p' is used as an expression and needs to be enclosed in
parentheses.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
 xen/arch/arm/include/asm/atomic.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/include/asm/atomic.h b/xen/arch/arm/include/asm/atomic.h
index ac2798d095..f5ef744b4b 100644
--- a/xen/arch/arm/include/asm/atomic.h
+++ b/xen/arch/arm/include/asm/atomic.h
@@ -123,15 +123,15 @@ static always_inline void write_atomic_size(volatile void *p,
 }
 
 #define read_atomic(p) ({                                               \
-    union { typeof(*p) val; char c[0]; } x_;                            \
-    read_atomic_size(p, x_.c, sizeof(*p));                              \
+    union { typeof(*(p)) val; char c[0]; } x_;                          \
+    read_atomic_size((p), x_.c, sizeof(*(p)));                          \
     x_.val;                                                             \
 })
 
 #define write_atomic(p, x)                                              \
     do {                                                                \
-        typeof(*p) x_ = (x);                                            \
-        write_atomic_size(p, &x_, sizeof(*p));                          \
+        typeof(*(p)) x_ = (x);                                          \
+        write_atomic_size((p), &x_, sizeof(*(p)));                      \
     } while ( false )
 
 #define add_sized(p, x) ({                                              \
-- 
2.34.1

On 27.07.2022 17:32, Xenia Ragiadakou wrote:
> The macro parameter 'p' is used as an expression and needs to be enclosed in
> parentheses.

Yes, but ...

> --- a/xen/arch/arm/include/asm/atomic.h
> +++ b/xen/arch/arm/include/asm/atomic.h
> @@ -123,15 +123,15 @@ static always_inline void write_atomic_size(volatile void *p,
>  }
>  
>  #define read_atomic(p) ({                                               \
> -    union { typeof(*p) val; char c[0]; } x_;                            \
> -    read_atomic_size(p, x_.c, sizeof(*p));                              \
> +    union { typeof(*(p)) val; char c[0]; } x_;                          \
> +    read_atomic_size((p), x_.c, sizeof(*(p)));                          \

... not in the first argument's case - that's not an expression.
Too few parentheses are a risk, but too many are as well, as they
negatively affect readability.

Jan

Hi Jan,

On 7/27/22 18:36, Jan Beulich wrote:
> On 27.07.2022 17:32, Xenia Ragiadakou wrote:
>> The macro parameter 'p' is used as an expression and needs to be enclosed in
>> parentheses.
> 
> Yes, but ...
> 
>> --- a/xen/arch/arm/include/asm/atomic.h
>> +++ b/xen/arch/arm/include/asm/atomic.h
>> @@ -123,15 +123,15 @@ static always_inline void write_atomic_size(volatile void *p,
>>   }
>>   
>>   #define read_atomic(p) ({                                               \
>> -    union { typeof(*p) val; char c[0]; } x_;                            \
>> -    read_atomic_size(p, x_.c, sizeof(*p));                              \
>> +    union { typeof(*(p)) val; char c[0]; } x_;                          \
>> +    read_atomic_size((p), x_.c, sizeof(*(p)));                          \
> 
> ... not in the first argument's case - that's not an expression.
> Too few parentheses are a risk, but too many are as well, as they
> negatively affect readability.
> 

Yes you are right. Here
write_atomic_size((p), &x_, sizeof(*(p)));
as well.

I will fix and resend.

-- 
Xenia