[XEN PATCH] atomic: change parameter name in atomic_cmpxchg() definition

Federico Serafini posted 1 patch 9 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/cace061a0b574d56f9b12a43a7c82276ef56654f.1689953642.git.federico.serafini@bugseng.com
xen/arch/arm/include/asm/arm64/atomic.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[XEN PATCH] atomic: change parameter name in atomic_cmpxchg() definition
Posted by Federico Serafini 9 months, 1 week ago
Change parameter name from 'ptr' to 'v' in the function definition thus
addressing violations of MISRA C:2012 Rule 8.3: "All declarations of an
object or function shall use the same names and type qualifiers".

No functional changes.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 xen/arch/arm/include/asm/arm64/atomic.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/include/asm/arm64/atomic.h b/xen/arch/arm/include/asm/arm64/atomic.h
index 2d42567866..4460165295 100644
--- a/xen/arch/arm/include/asm/arm64/atomic.h
+++ b/xen/arch/arm/include/asm/arm64/atomic.h
@@ -105,7 +105,7 @@ static inline void atomic_and(int m, atomic_t *v)
 	: "Ir" (m));
 }
 
-static inline int atomic_cmpxchg(atomic_t *ptr, int old, int new)
+static inline int atomic_cmpxchg(atomic_t *v, int old, int new)
 {
 	unsigned long tmp;
 	int oldval;
@@ -119,7 +119,7 @@ static inline int atomic_cmpxchg(atomic_t *ptr, int old, int new)
 "	stxr	%w0, %w4, %2\n"
 "	cbnz	%w0, 1b\n"
 "2:"
-	: "=&r" (tmp), "=&r" (oldval), "+Q" (ptr->counter)
+	: "=&r" (tmp), "=&r" (oldval), "+Q" (v->counter)
 	: "Ir" (old), "r" (new)
 	: "cc");
 
-- 
2.34.1
Re: [XEN PATCH] atomic: change parameter name in atomic_cmpxchg() definition
Posted by Julien Grall 9 months, 1 week ago
Hi,

Title:

Please add "xen/arm: " to clarify this is a patch touching the Arm code.

On 21/07/2023 16:37, Federico Serafini wrote:
> Change parameter name from 'ptr' to 'v' in the function definition thus
> addressing violations of MISRA C:2012 Rule 8.3: "All declarations of an
> object or function shall use the same names and type qualifiers".

The parameters are consistent between arm32 and arm64. Naming wise, any 
reason you picked the x86 name? Personally, I have a slight preference 
to keep 'ptr' because this is more obvious than 'v'.

But I will not strongly argue against it. That said, if you are looking 
for consistency, you should also modify arm32.

Cheers,

-- 
Julien Grall
Re: [XEN PATCH] atomic: change parameter name in atomic_cmpxchg() definition
Posted by Federico Serafini 9 months, 1 week ago
Hello Julien,

On 21/07/23 17:44, Julien Grall wrote:> Title:
> 
> Please add "xen/arm: " to clarify this is a patch touching the Arm code.

Ok.

> On 21/07/2023 16:37, Federico Serafini wrote:
>> Change parameter name from 'ptr' to 'v' in the function definition thus
>> addressing violations of MISRA C:2012 Rule 8.3: "All declarations of an
>> object or function shall use the same names and type qualifiers".
> 
> The parameters are consistent between arm32 and arm64. Naming wise, any 
> reason you picked the x86 name? Personally, I have a slight preference 
> to keep 'ptr' because this is more obvious than 'v'.
I picked 'v' because in xen/arch/arm/include/asm/arm64/atomic.h,
all the parameters having 'atomic_t *' type are named in that way.
The same is true for xen/include/xen/atomic.h.

If you prefer I can go for 'ptr'.

> But I will not strongly argue against it. That said, if you are looking 
> for consistency, you should also modify arm32.
> 
> Cheers,

After deciding what to do, I will propagate the change to arm32.

Regards
-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)
Re: [XEN PATCH] atomic: change parameter name in atomic_cmpxchg() definition
Posted by Julien Grall 9 months, 1 week ago
On 21/07/2023 17:15, Federico Serafini wrote:
> Hello Julien,

Hi,

> On 21/07/23 17:44, Julien Grall wrote:> Title:
>>
>> Please add "xen/arm: " to clarify this is a patch touching the Arm code.
> 
> Ok.
> 
>> On 21/07/2023 16:37, Federico Serafini wrote:
>>> Change parameter name from 'ptr' to 'v' in the function definition thus
>>> addressing violations of MISRA C:2012 Rule 8.3: "All declarations of an
>>> object or function shall use the same names and type qualifiers".
>>
>> The parameters are consistent between arm32 and arm64. Naming wise, 
>> any reason you picked the x86 name? Personally, I have a slight 
>> preference to keep 'ptr' because this is more obvious than 'v'.
> I picked 'v' because in xen/arch/arm/include/asm/arm64/atomic.h,
> all the parameters having 'atomic_t *' type are named in that way.
> The same is true for xen/include/xen/atomic.h.

Thanks for the information.

> If you prefer I can go for 'ptr'.

But that would mean changing x86 / common. As you said the other helpers 
are also using 'v'. So I am not sure this is really worth the naming 
argument. Which is why I said I will not strongly argue for it. In fact, 
the only reason I didn't approve this patch is because...

> 
>> But I will not strongly argue against it. That said, if you are 
>> looking for consistency, you should also modify arm32.
>>
>> Cheers,
> 
> After deciding what to do, I will propagate the change to arm32.

... you didn't modify arm32. I should have made it clearer, sorry.

Cheers,

-- 
Julien Grall