On 6/5/25 02:31, Jan Beulich wrote:
> On 05.06.2025 01:35, Stefano Stabellini wrote:
>> From: Alessandro Zucchelli <alessandro.zucche...@bugseng.com>
>>
>> MISRA C Rule 21.16 states the following: "The pointer arguments to
>> the Standard Library function `memcmp' shall point to either a pointer
>> type, an essentially signed type, an essentially unsigned type, an
>> essentially Boolean type or an essentially enum type".
>>
>> Comparing string literals with char arrays is more appropriately
>> done via strncmp.
>
> More appropriately - maybe. Yet less efficiently. IOW I view ...
>
>> No functional change.
>
> ... this as at the edge of not being true.
>
>> Signed-off-by: Alessandro Zucchelli <alessandro.zucche...@bugseng.com>
>
> Missing your own S-o-b.
>
> Also (nit) may I ask that you drop the full stop from the patch subject?
>
>> --- a/xen/arch/x86/dmi_scan.c
>> +++ b/xen/arch/x86/dmi_scan.c
>> @@ -233,7 +233,7 @@ void __init dmi_efi_get_table(const void *smbios, const
>> void *smbios3)
>> const struct smbios_eps *eps = smbios;
>> const struct smbios3_eps *eps3 = smbios3;
>>
>> - if (eps3 && memcmp(eps3->anchor, "_SM3_", 5) == 0 &&
>> + if (eps3 && strncmp(eps3->anchor, "_SM3_", 5) == 0 &&
>
> Unlike the last example given in the doc, this does not pose the risk of
> false "not equal" returns. Considering there's no example there exactly
> matching this situation, I'm not convinced a change is actually needed.
> (Applies to all other changes here, too.)
>
>> @@ -302,7 +302,7 @@ const char *__init dmi_get_table(paddr_t *base, u32 *len)
>> continue;
>> memcpy_fromio(&eps.dmi + 1, q + sizeof(eps.dmi),
>> sizeof(eps.smbios3) - sizeof(eps.dmi));
>> - if (!memcmp(eps.smbios3.anchor, "_SM3_", 5) &&
>> + if (strncmp(eps.smbios3.anchor, "_SM3_", 5) == 0 &&
>
> Here and below there's a further (style) change, moving from ! to "== 0"
> (or from implicit boolean to "!= 0"). As we use the original style in many
> other places, some justification for this extra change would be needed in
> the description (or these extra adjustments be dropped).
>
>> @@ -720,10 +720,10 @@ static void __init efi_check_config(void)
>> __set_fixmap(FIX_EFI_MPF, PFN_DOWN(efi.mps), __PAGE_HYPERVISOR);
>> mpf = fix_to_virt(FIX_EFI_MPF) + ((long)efi.mps & (PAGE_SIZE-1));
>>
>> - if (memcmp(mpf->mpf_signature, "_MP_", 4) == 0 &&
>> - mpf->mpf_length == 1 &&
>> - mpf_checksum((void *)mpf, 16) &&
>> - (mpf->mpf_specification == 1 || mpf->mpf_specification == 4)) {
>> + if (strncmp(mpf->mpf_signature, "_MP_", 4) == 0 &&
>> + mpf->mpf_length == 1 &&
>> + mpf_checksum((void *)mpf, 16) &&
>> + (mpf->mpf_specification == 1 || mpf->mpf_specification == 4)) {
>> smp_found_config = true;
>> printk(KERN_INFO "SMP MP-table at %08lx\n", efi.mps);
>> mpf_found = mpf;
>
> There are extra (indentation) changes here which ought to be dropped.What about using an array of const uint8_t[] instead of a string literal? -- Sincerely, Demi Marie Obenour (she/her/hers)
OpenPGP_0xB288B55FFF9C22C1.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
