[PATCH 9/9] drivers/acpi: Use explicitly specified types

Michal Orzel posted 9 patches 3 years, 7 months ago
[PATCH 9/9] drivers/acpi: Use explicitly specified types
Posted by Michal Orzel 3 years, 7 months ago
According to MISRA C 2012 Rule 8.1, types shall be explicitly
specified. Fix all the findings reported by cppcheck with misra addon
by substituting implicit type 'unsigned' to explicit 'unsigned int'.

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
This patch may not be applicable as these files come from Linux.
---
 xen/drivers/acpi/tables/tbfadt.c  | 2 +-
 xen/drivers/acpi/tables/tbutils.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/acpi/tables/tbfadt.c b/xen/drivers/acpi/tables/tbfadt.c
index f11fd5a900..ad55cd769a 100644
--- a/xen/drivers/acpi/tables/tbfadt.c
+++ b/xen/drivers/acpi/tables/tbfadt.c
@@ -235,7 +235,7 @@ void __init acpi_tb_create_local_fadt(struct acpi_table_header *table, u32 lengt
 		ACPI_WARNING((AE_INFO,
 			      "FADT (revision %u) is longer than ACPI 5.0 version,"
 			      " truncating length %u to %zu",
-			      table->revision, (unsigned)length,
+			      table->revision, (unsigned int)length,
 			      sizeof(struct acpi_table_fadt)));
 	}
 
diff --git a/xen/drivers/acpi/tables/tbutils.c b/xen/drivers/acpi/tables/tbutils.c
index d135a50ff9..ddb7f628c9 100644
--- a/xen/drivers/acpi/tables/tbutils.c
+++ b/xen/drivers/acpi/tables/tbutils.c
@@ -481,7 +481,7 @@ acpi_tb_parse_root_table(acpi_physical_address rsdp_address, u8 flags)
 			if (ACPI_FAILURE(status)) {
 				ACPI_WARNING((AE_INFO,
 					      "Truncating %u table entries!",
-					      (unsigned)
+					      (unsigned int)
 					      (acpi_gbl_root_table_list.size -
 					       acpi_gbl_root_table_list.
 					       count)));
-- 
2.25.1
Re: [PATCH 9/9] drivers/acpi: Use explicitly specified types
Posted by Jan Beulich 3 years, 7 months ago
On 20.06.2022 09:02, Michal Orzel wrote:
> According to MISRA C 2012 Rule 8.1, types shall be explicitly
> specified. Fix all the findings reported by cppcheck with misra addon
> by substituting implicit type 'unsigned' to explicit 'unsigned int'.
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
> ---
> This patch may not be applicable as these files come from Linux.

We've diverged quite far, so the Linux origin isn't really a concern
(anymore).

> --- a/xen/drivers/acpi/tables/tbfadt.c
> +++ b/xen/drivers/acpi/tables/tbfadt.c
> @@ -235,7 +235,7 @@ void __init acpi_tb_create_local_fadt(struct acpi_table_header *table, u32 lengt
>  		ACPI_WARNING((AE_INFO,
>  			      "FADT (revision %u) is longer than ACPI 5.0 version,"
>  			      " truncating length %u to %zu",
> -			      table->revision, (unsigned)length,
> +			      table->revision, (unsigned int)length,

Since we generally try to avoid casts where not needed - did you consider
dropping the cast here instead of fiddling with it? Aiui this wouldn't be
a problem since we assume sizeof(int) >= 4 and since types more narrow
than int would be converted to int (which in turn is generally printing
okay with %u). Strictly speaking it would want to be PRIu32 ...

> --- a/xen/drivers/acpi/tables/tbutils.c
> +++ b/xen/drivers/acpi/tables/tbutils.c
> @@ -481,7 +481,7 @@ acpi_tb_parse_root_table(acpi_physical_address rsdp_address, u8 flags)
>  			if (ACPI_FAILURE(status)) {
>  				ACPI_WARNING((AE_INFO,
>  					      "Truncating %u table entries!",
> -					      (unsigned)
> +					      (unsigned int)
>  					      (acpi_gbl_root_table_list.size -
>  					       acpi_gbl_root_table_list.
>  					       count)));

Same here then, except PRIu32 wouldn't be correct to use in this case.

Jan
Re: [PATCH 9/9] drivers/acpi: Use explicitly specified types
Posted by Michal Orzel 3 years, 7 months ago

On 22.06.2022 12:36, Jan Beulich wrote:
> On 20.06.2022 09:02, Michal Orzel wrote:
>> According to MISRA C 2012 Rule 8.1, types shall be explicitly
>> specified. Fix all the findings reported by cppcheck with misra addon
>> by substituting implicit type 'unsigned' to explicit 'unsigned int'.
>>
>> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
>> ---
>> This patch may not be applicable as these files come from Linux.
> 
> We've diverged quite far, so the Linux origin isn't really a concern
> (anymore).
> 
Ok.
>> --- a/xen/drivers/acpi/tables/tbfadt.c
>> +++ b/xen/drivers/acpi/tables/tbfadt.c
>> @@ -235,7 +235,7 @@ void __init acpi_tb_create_local_fadt(struct acpi_table_header *table, u32 lengt
>>  		ACPI_WARNING((AE_INFO,
>>  			      "FADT (revision %u) is longer than ACPI 5.0 version,"
>>  			      " truncating length %u to %zu",
>> -			      table->revision, (unsigned)length,
>> +			      table->revision, (unsigned int)length,
> 
> Since we generally try to avoid casts where not needed - did you consider
> dropping the cast here instead of fiddling with it? Aiui this wouldn't be
> a problem since we assume sizeof(int) >= 4 and since types more narrow
> than int would be converted to int (which in turn is generally printing
> okay with %u). Strictly speaking it would want to be PRIu32 ...
> 
The reason I did not consider it was to make the review process easier by performing only
the mechanical change being part of fixing 8.1 rule. However I'm fully ok to drop the cast.

Changing the format specifier to PRIu32 for length would require using PRIu8 for table->revision
to be coherent.

>> --- a/xen/drivers/acpi/tables/tbutils.c
>> +++ b/xen/drivers/acpi/tables/tbutils.c
>> @@ -481,7 +481,7 @@ acpi_tb_parse_root_table(acpi_physical_address rsdp_address, u8 flags)
>>  			if (ACPI_FAILURE(status)) {
>>  				ACPI_WARNING((AE_INFO,
>>  					      "Truncating %u table entries!",
>> -					      (unsigned)
>> +					      (unsigned int)
>>  					      (acpi_gbl_root_table_list.size -
>>  					       acpi_gbl_root_table_list.
>>  					       count)));
> 
> Same here then, except PRIu32 wouldn't be correct to use in this case.
> 
Why is it so given that both size and count are of type u32?

> Jan

Cheers,
Michal
Re: [PATCH 9/9] drivers/acpi: Use explicitly specified types
Posted by Jan Beulich 3 years, 7 months ago
On 22.06.2022 13:09, Michal Orzel wrote:
> On 22.06.2022 12:36, Jan Beulich wrote:
>> On 20.06.2022 09:02, Michal Orzel wrote:
>>> --- a/xen/drivers/acpi/tables/tbutils.c
>>> +++ b/xen/drivers/acpi/tables/tbutils.c
>>> @@ -481,7 +481,7 @@ acpi_tb_parse_root_table(acpi_physical_address rsdp_address, u8 flags)
>>>  			if (ACPI_FAILURE(status)) {
>>>  				ACPI_WARNING((AE_INFO,
>>>  					      "Truncating %u table entries!",
>>> -					      (unsigned)
>>> +					      (unsigned int)
>>>  					      (acpi_gbl_root_table_list.size -
>>>  					       acpi_gbl_root_table_list.
>>>  					       count)));
>>
>> Same here then, except PRIu32 wouldn't be correct to use in this case.
>>
> Why is it so given that both size and count are of type u32?

Because the promoted type (i.e. the type of the result of the subtraction)
isn't uint32_t (and will never be). It'll be "unsigned int" when
sizeof(int) == 4 (and in this case it'll happen to alias uint32_t) and
just "int" when sizeof(int) > 4 (not even aliasing int32_t).

Jan