[XEN PATCH v2 06/13] xen/efi: fix violations of MISRA C:2012 Rule 7.2

Simone Ballarin posted 13 patches 2 years, 7 months ago
There is a newer version of this series
[XEN PATCH v2 06/13] xen/efi: fix violations of MISRA C:2012 Rule 7.2
Posted by Simone Ballarin 2 years, 7 months ago
From: Gianluca Luparini <gianluca.luparini@bugseng.com>

The xen sources contains violations of MISRA C:2012 Rule 7.2 whose
headline states:
"A 'u' or 'U' suffix shall be applied to all integer constants
that are represented in an unsigned type".

Add the 'U' suffix to integers literals with unsigned type and also to other
literals used in the same contexts or near violations, when their positive
nature is immediately clear. The latter changes are done for the sake of
uniformity.

Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
Signed-off-by: Gianluca Luparini <gianluca.luparini@bugseng.com>
---
Changes in v2:
- minor change to commit title
- change commit message
- remove changes in 'efibind.h', 'efiapi.h', 'efidef.h' and 'efiprot.h'
---
 xen/common/efi/boot.c    | 8 ++++----
 xen/common/efi/runtime.c | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index c5850c26af..28a57fe434 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -34,13 +34,13 @@
 #define EFI_REVISION(major, minor) (((major) << 16) | (minor))
 
 #define SMBIOS3_TABLE_GUID \
-  { 0xf2fd1544, 0x9794, 0x4a2c, {0x99, 0x2e, 0xe5, 0xbb, 0xcf, 0x20, 0xe3, 0x94} }
+  { 0xf2fd1544U, 0x9794U, 0x4a2cU, {0x99U, 0x2eU, 0xe5U, 0xbbU, 0xcfU, 0x20U, 0xe3U, 0x94U} }
 #define SHIM_LOCK_PROTOCOL_GUID \
-  { 0x605dab50, 0xe046, 0x4300, {0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23} }
+  { 0x605dab50U, 0xe046U, 0x4300U, {0xabU, 0xb6U, 0x3dU, 0xd8U, 0x10U, 0xddU, 0x8bU, 0x23U} }
 #define APPLE_PROPERTIES_PROTOCOL_GUID \
-  { 0x91bd12fe, 0xf6c3, 0x44fb, { 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0} }
+  { 0x91bd12feU, 0xf6c3U, 0x44fbU, { 0xa5U, 0xb7U, 0x51U, 0x22U, 0xabU, 0x30U, 0x3aU, 0xe0U} }
 #define EFI_SYSTEM_RESOURCE_TABLE_GUID    \
-  { 0xb122a263, 0x3661, 0x4f68, {0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80} }
+  { 0xb122a263U, 0x3661U, 0x4f68U, {0x99U, 0x29U, 0x78U, 0xf8U, 0xb0U, 0xd6U, 0x21U, 0x80U} }
 #define EFI_SYSTEM_RESOURCE_TABLE_FIRMWARE_RESOURCE_VERSION 1
 
 typedef struct {
diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
index 13b0975866..5cb7504c96 100644
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -698,7 +698,7 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
 #ifndef COMPAT
     op->status = status;
 #else
-    op->status = (status & 0x3fffffff) | ((status >> 32) & 0xc0000000);
+    op->status = (status & 0x3fffffffU) | ((status >> 32) & 0xc0000000U);
 #endif
 
     return rc;
-- 
2.41.0
Re: [XEN PATCH v2 06/13] xen/efi: fix violations of MISRA C:2012 Rule 7.2
Posted by Stefano Stabellini 2 years, 7 months ago
On Wed, 5 Jul 2023, Simone Ballarin wrote:
> From: Gianluca Luparini <gianluca.luparini@bugseng.com>
> 
> The xen sources contains violations of MISRA C:2012 Rule 7.2 whose
> headline states:
> "A 'u' or 'U' suffix shall be applied to all integer constants
> that are represented in an unsigned type".
> 
> Add the 'U' suffix to integers literals with unsigned type and also to other
> literals used in the same contexts or near violations, when their positive
> nature is immediately clear. The latter changes are done for the sake of
> uniformity.
> 
> Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
> Signed-off-by: Gianluca Luparini <gianluca.luparini@bugseng.com>
>
> ---
> Changes in v2:
> - minor change to commit title
> - change commit message
> - remove changes in 'efibind.h', 'efiapi.h', 'efidef.h' and 'efiprot.h'
> ---
>  xen/common/efi/boot.c    | 8 ++++----
>  xen/common/efi/runtime.c | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index c5850c26af..28a57fe434 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -34,13 +34,13 @@
>  #define EFI_REVISION(major, minor) (((major) << 16) | (minor))
>  
>  #define SMBIOS3_TABLE_GUID \
> -  { 0xf2fd1544, 0x9794, 0x4a2c, {0x99, 0x2e, 0xe5, 0xbb, 0xcf, 0x20, 0xe3, 0x94} }
> +  { 0xf2fd1544U, 0x9794U, 0x4a2cU, {0x99U, 0x2eU, 0xe5U, 0xbbU, 0xcfU, 0x20U, 0xe3U, 0x94U} }
>  #define SHIM_LOCK_PROTOCOL_GUID \
> -  { 0x605dab50, 0xe046, 0x4300, {0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23} }
> +  { 0x605dab50U, 0xe046U, 0x4300U, {0xabU, 0xb6U, 0x3dU, 0xd8U, 0x10U, 0xddU, 0x8bU, 0x23U} }
>  #define APPLE_PROPERTIES_PROTOCOL_GUID \
> -  { 0x91bd12fe, 0xf6c3, 0x44fb, { 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0} }
> +  { 0x91bd12feU, 0xf6c3U, 0x44fbU, { 0xa5U, 0xb7U, 0x51U, 0x22U, 0xabU, 0x30U, 0x3aU, 0xe0U} }
>  #define EFI_SYSTEM_RESOURCE_TABLE_GUID    \
> -  { 0xb122a263, 0x3661, 0x4f68, {0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80} }
> +  { 0xb122a263U, 0x3661U, 0x4f68U, {0x99U, 0x29U, 0x78U, 0xf8U, 0xb0U, 0xd6U, 0x21U, 0x80U} }
>  #define EFI_SYSTEM_RESOURCE_TABLE_FIRMWARE_RESOURCE_VERSION 1

To be honest in these case I don't know if this is better or if it is
better to add U only to the first numbers (0xf2fd1544U, 0x605dab50U,
0x91bd12feU, 0xb122a263U).

Either way:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>



>  typedef struct {
> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
> index 13b0975866..5cb7504c96 100644
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -698,7 +698,7 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
>  #ifndef COMPAT
>      op->status = status;
>  #else
> -    op->status = (status & 0x3fffffff) | ((status >> 32) & 0xc0000000);
> +    op->status = (status & 0x3fffffffU) | ((status >> 32) & 0xc0000000U);
>  #endif
>  
>      return rc;
> -- 
> 2.41.0
>
Re: [XEN PATCH v2 06/13] xen/efi: fix violations of MISRA C:2012 Rule 7.2
Posted by Jan Beulich 2 years, 7 months ago
On 06.07.2023 01:37, Stefano Stabellini wrote:
> On Wed, 5 Jul 2023, Simone Ballarin wrote:
>> --- a/xen/common/efi/boot.c
>> +++ b/xen/common/efi/boot.c
>> @@ -34,13 +34,13 @@
>>  #define EFI_REVISION(major, minor) (((major) << 16) | (minor))
>>  
>>  #define SMBIOS3_TABLE_GUID \
>> -  { 0xf2fd1544, 0x9794, 0x4a2c, {0x99, 0x2e, 0xe5, 0xbb, 0xcf, 0x20, 0xe3, 0x94} }
>> +  { 0xf2fd1544U, 0x9794U, 0x4a2cU, {0x99U, 0x2eU, 0xe5U, 0xbbU, 0xcfU, 0x20U, 0xe3U, 0x94U} }
>>  #define SHIM_LOCK_PROTOCOL_GUID \
>> -  { 0x605dab50, 0xe046, 0x4300, {0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23} }
>> +  { 0x605dab50U, 0xe046U, 0x4300U, {0xabU, 0xb6U, 0x3dU, 0xd8U, 0x10U, 0xddU, 0x8bU, 0x23U} }
>>  #define APPLE_PROPERTIES_PROTOCOL_GUID \
>> -  { 0x91bd12fe, 0xf6c3, 0x44fb, { 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0} }
>> +  { 0x91bd12feU, 0xf6c3U, 0x44fbU, { 0xa5U, 0xb7U, 0x51U, 0x22U, 0xabU, 0x30U, 0x3aU, 0xe0U} }
>>  #define EFI_SYSTEM_RESOURCE_TABLE_GUID    \
>> -  { 0xb122a263, 0x3661, 0x4f68, {0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80} }
>> +  { 0xb122a263U, 0x3661U, 0x4f68U, {0x99U, 0x29U, 0x78U, 0xf8U, 0xb0U, 0xd6U, 0x21U, 0x80U} }
>>  #define EFI_SYSTEM_RESOURCE_TABLE_FIRMWARE_RESOURCE_VERSION 1
> 
> To be honest in these case I don't know if this is better or if it is
> better to add U only to the first numbers (0xf2fd1544U, 0x605dab50U,
> 0x91bd12feU, 0xb122a263U).

I'd prefer the latter, but I wonder whether that's in the spirit of Misra.

Jan
Re: [XEN PATCH v2 06/13] xen/efi: fix violations of MISRA C:2012 Rule 7.2
Posted by Stefano Stabellini 2 years, 7 months ago
On Thu, 6 Jul 2023, Jan Beulich wrote:
> On 06.07.2023 01:37, Stefano Stabellini wrote:
> > On Wed, 5 Jul 2023, Simone Ballarin wrote:
> >> --- a/xen/common/efi/boot.c
> >> +++ b/xen/common/efi/boot.c
> >> @@ -34,13 +34,13 @@
> >>  #define EFI_REVISION(major, minor) (((major) << 16) | (minor))
> >>  
> >>  #define SMBIOS3_TABLE_GUID \
> >> -  { 0xf2fd1544, 0x9794, 0x4a2c, {0x99, 0x2e, 0xe5, 0xbb, 0xcf, 0x20, 0xe3, 0x94} }
> >> +  { 0xf2fd1544U, 0x9794U, 0x4a2cU, {0x99U, 0x2eU, 0xe5U, 0xbbU, 0xcfU, 0x20U, 0xe3U, 0x94U} }
> >>  #define SHIM_LOCK_PROTOCOL_GUID \
> >> -  { 0x605dab50, 0xe046, 0x4300, {0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23} }
> >> +  { 0x605dab50U, 0xe046U, 0x4300U, {0xabU, 0xb6U, 0x3dU, 0xd8U, 0x10U, 0xddU, 0x8bU, 0x23U} }
> >>  #define APPLE_PROPERTIES_PROTOCOL_GUID \
> >> -  { 0x91bd12fe, 0xf6c3, 0x44fb, { 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0} }
> >> +  { 0x91bd12feU, 0xf6c3U, 0x44fbU, { 0xa5U, 0xb7U, 0x51U, 0x22U, 0xabU, 0x30U, 0x3aU, 0xe0U} }
> >>  #define EFI_SYSTEM_RESOURCE_TABLE_GUID    \
> >> -  { 0xb122a263, 0x3661, 0x4f68, {0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80} }
> >> +  { 0xb122a263U, 0x3661U, 0x4f68U, {0x99U, 0x29U, 0x78U, 0xf8U, 0xb0U, 0xd6U, 0x21U, 0x80U} }
> >>  #define EFI_SYSTEM_RESOURCE_TABLE_FIRMWARE_RESOURCE_VERSION 1
> > 
> > To be honest in these case I don't know if this is better or if it is
> > better to add U only to the first numbers (0xf2fd1544U, 0x605dab50U,
> > 0x91bd12feU, 0xb122a263U).
> 
> I'd prefer the latter, but I wonder whether that's in the spirit of Misra.

My understanding is that MISRA only requests U to be added where
actually needed. Everything else is up to us and our desire (or not) for
consistency.
Re: [XEN PATCH v2 06/13] xen/efi: fix violations of MISRA C:2012 Rule 7.2
Posted by Luca Fancellu 2 years, 7 months ago

> On 5 Jul 2023, at 16:26, Simone Ballarin <simone.ballarin@bugseng.com> wrote:
> 
> From: Gianluca Luparini <gianluca.luparini@bugseng.com>
> 
> The xen sources contains violations of MISRA C:2012 Rule 7.2 whose
> headline states:
> "A 'u' or 'U' suffix shall be applied to all integer constants
> that are represented in an unsigned type".
> 
> Add the 'U' suffix to integers literals with unsigned type and also to other
> literals used in the same contexts or near violations, when their positive
> nature is immediately clear. The latter changes are done for the sake of
> uniformity.
> 
> Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
> Signed-off-by: Gianluca Luparini <gianluca.luparini@bugseng.com>

Changes looks good to me, probably Jan knows better this part

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

> ---
> Changes in v2:
> - minor change to commit title
> - change commit message
> - remove changes in 'efibind.h', 'efiapi.h', 'efidef.h' and 'efiprot.h'
> ---
> xen/common/efi/boot.c    | 8 ++++----
> xen/common/efi/runtime.c | 2 +-
> 2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index c5850c26af..28a57fe434 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -34,13 +34,13 @@
> #define EFI_REVISION(major, minor) (((major) << 16) | (minor))
> 
> #define SMBIOS3_TABLE_GUID \
> -  { 0xf2fd1544, 0x9794, 0x4a2c, {0x99, 0x2e, 0xe5, 0xbb, 0xcf, 0x20, 0xe3, 0x94} }
> +  { 0xf2fd1544U, 0x9794U, 0x4a2cU, {0x99U, 0x2eU, 0xe5U, 0xbbU, 0xcfU, 0x20U, 0xe3U, 0x94U} }
> #define SHIM_LOCK_PROTOCOL_GUID \
> -  { 0x605dab50, 0xe046, 0x4300, {0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23} }
> +  { 0x605dab50U, 0xe046U, 0x4300U, {0xabU, 0xb6U, 0x3dU, 0xd8U, 0x10U, 0xddU, 0x8bU, 0x23U} }
> #define APPLE_PROPERTIES_PROTOCOL_GUID \
> -  { 0x91bd12fe, 0xf6c3, 0x44fb, { 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0} }
> +  { 0x91bd12feU, 0xf6c3U, 0x44fbU, { 0xa5U, 0xb7U, 0x51U, 0x22U, 0xabU, 0x30U, 0x3aU, 0xe0U} }
> #define EFI_SYSTEM_RESOURCE_TABLE_GUID    \
> -  { 0xb122a263, 0x3661, 0x4f68, {0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80} }
> +  { 0xb122a263U, 0x3661U, 0x4f68U, {0x99U, 0x29U, 0x78U, 0xf8U, 0xb0U, 0xd6U, 0x21U, 0x80U} }
> #define EFI_SYSTEM_RESOURCE_TABLE_FIRMWARE_RESOURCE_VERSION 1
> 
> typedef struct {
> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
> index 13b0975866..5cb7504c96 100644
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -698,7 +698,7 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
> #ifndef COMPAT
>     op->status = status;
> #else
> -    op->status = (status & 0x3fffffff) | ((status >> 32) & 0xc0000000);
> +    op->status = (status & 0x3fffffffU) | ((status >> 32) & 0xc0000000U);
> #endif
> 
>     return rc;
> -- 
> 2.41.0
> 
>