diff mbox series

[4/9] ACPI: address violations of MISRA C:2012 Rule 11.8

Message ID 44c8f94bcfe4f0e33e53a7eb8aef826e7d906196.1702555387.git.maria.celeste.cesario@bugseng.com (mailing list archive)
State Superseded
Headers show
Series xen: address violations of MISRA C:2012 Rule 11.8 | expand

Commit Message

Simone Ballarin Dec. 14, 2023, 12:07 p.m. UTC
From: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>

The xen sources contain violations of MISRA C:2012 Rule 11.8 whose
headline states:
"A conversion shall not remove any const, volatile or _Atomic qualification
from the type pointed to by a pointer".

Add missing const qualifiers missing in casting.
There's no reason to drop the const qualifier in ACPI_COMPARE_NAME since
the macro arguments are not modified in its body.

Signed-off-by: Maria Celeste Cesario  <maria.celeste.cesario@bugseng.com>
Signed-off-by: Simone Ballarin  <simone.ballarin@bugseng.com>
---
 xen/include/acpi/acmacros.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jan Beulich Dec. 14, 2023, 4:36 p.m. UTC | #1
On 14.12.2023 13:07, Simone Ballarin wrote:
> --- a/xen/include/acpi/acmacros.h
> +++ b/xen/include/acpi/acmacros.h
> @@ -116,7 +116,7 @@
>  #define ACPI_PTR_TO_PHYSADDR(i)         ACPI_TO_INTEGER(i)
>  
>  #ifndef ACPI_MISALIGNMENT_NOT_SUPPORTED
> -#define ACPI_COMPARE_NAME(a,b)          (*ACPI_CAST_PTR (u32,(a)) == *ACPI_CAST_PTR (u32,(b)))
> +#define ACPI_COMPARE_NAME(a,b)          (*ACPI_CAST_PTR (const u32,(a)) == *ACPI_CAST_PTR (const u32,(b)))

Hmm, I'm a little hesitant to take changes to this header. We've
inherited it from Linux, who in turn inherited / imported it from
ACPI CA.

>  #else
>  #define ACPI_COMPARE_NAME(a,b)          (!ACPI_STRNCMP (ACPI_CAST_PTR (char,(a)), ACPI_CAST_PTR (char,(b)), ACPI_NAME_SIZE))
>  #endif

What about this alternative code, btw?

Jan
Stefano Stabellini Dec. 14, 2023, 9:49 p.m. UTC | #2
On Thu, 14 Dec 2023, Simone Ballarin wrote:
> From: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
> 
> The xen sources contain violations of MISRA C:2012 Rule 11.8 whose
> headline states:
> "A conversion shall not remove any const, volatile or _Atomic qualification
> from the type pointed to by a pointer".
> 
> Add missing const qualifiers missing in casting.
> There's no reason to drop the const qualifier in ACPI_COMPARE_NAME since
> the macro arguments are not modified in its body.
> 
> Signed-off-by: Maria Celeste Cesario  <maria.celeste.cesario@bugseng.com>
> Signed-off-by: Simone Ballarin  <simone.ballarin@bugseng.com>
> ---
>  xen/include/acpi/acmacros.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/include/acpi/acmacros.h b/xen/include/acpi/acmacros.h
> index 86c503c20f..d7c74c5769 100644
> --- a/xen/include/acpi/acmacros.h
> +++ b/xen/include/acpi/acmacros.h
> @@ -116,7 +116,7 @@
>  #define ACPI_PTR_TO_PHYSADDR(i)         ACPI_TO_INTEGER(i)
>  
>  #ifndef ACPI_MISALIGNMENT_NOT_SUPPORTED
> -#define ACPI_COMPARE_NAME(a,b)          (*ACPI_CAST_PTR (u32,(a)) == *ACPI_CAST_PTR (u32,(b)))
> +#define ACPI_COMPARE_NAME(a,b)          (*ACPI_CAST_PTR (const u32,(a)) == *ACPI_CAST_PTR (const u32,(b)))

I am a bit confused but this one. The implementation of ACPI_CAST_PTR
is:

#define ACPI_CAST_PTR(t, p)             ((t *) (acpi_uintptr_t) (p))

The first cast to acpi_uintptr_t would already drop the const anyway?


>  #else
>  #define ACPI_COMPARE_NAME(a,b)          (!ACPI_STRNCMP (ACPI_CAST_PTR (char,(a)), ACPI_CAST_PTR (char,(b)), ACPI_NAME_SIZE))
>  #endif

Would it make sense to also add the const here too if nothing else for
consistency?
Simone Ballarin Dec. 18, 2023, 3:05 p.m. UTC | #3
On 14/12/23 17:36, Jan Beulich wrote:
> On 14.12.2023 13:07, Simone Ballarin wrote:
>> --- a/xen/include/acpi/acmacros.h
>> +++ b/xen/include/acpi/acmacros.h
>> @@ -116,7 +116,7 @@
>>   #define ACPI_PTR_TO_PHYSADDR(i)         ACPI_TO_INTEGER(i)
>>   
>>   #ifndef ACPI_MISALIGNMENT_NOT_SUPPORTED
>> -#define ACPI_COMPARE_NAME(a,b)          (*ACPI_CAST_PTR (u32,(a)) == *ACPI_CAST_PTR (u32,(b)))
>> +#define ACPI_COMPARE_NAME(a,b)          (*ACPI_CAST_PTR (const u32,(a)) == *ACPI_CAST_PTR (const u32,(b)))
> 
> Hmm, I'm a little hesitant to take changes to this header. We've
> inherited it from Linux, who in turn inherited / imported it from
> ACPI CA.
> 
>>   #else
>>   #define ACPI_COMPARE_NAME(a,b)          (!ACPI_STRNCMP (ACPI_CAST_PTR (char,(a)), ACPI_CAST_PTR (char,(b)), ACPI_NAME_SIZE))
>>   #endif
> 
> What about this alternative code, btw?
> 
> Jan
> 
If the file is not supposed to be changed, I would consider adding it in
docs/misra/exclude-list*.
For the moment, I think it is better to drop the change.
diff mbox series

Patch

diff --git a/xen/include/acpi/acmacros.h b/xen/include/acpi/acmacros.h
index 86c503c20f..d7c74c5769 100644
--- a/xen/include/acpi/acmacros.h
+++ b/xen/include/acpi/acmacros.h
@@ -116,7 +116,7 @@ 
 #define ACPI_PTR_TO_PHYSADDR(i)         ACPI_TO_INTEGER(i)
 
 #ifndef ACPI_MISALIGNMENT_NOT_SUPPORTED
-#define ACPI_COMPARE_NAME(a,b)          (*ACPI_CAST_PTR (u32,(a)) == *ACPI_CAST_PTR (u32,(b)))
+#define ACPI_COMPARE_NAME(a,b)          (*ACPI_CAST_PTR (const u32,(a)) == *ACPI_CAST_PTR (const u32,(b)))
 #else
 #define ACPI_COMPARE_NAME(a,b)          (!ACPI_STRNCMP (ACPI_CAST_PTR (char,(a)), ACPI_CAST_PTR (char,(b)), ACPI_NAME_SIZE))
 #endif