| Message ID | 20220620070245.77979-10-michal.orzel@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MISRA C 2012 8.1 rule fixes | expand |
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
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
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
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)));
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(-)