[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86: remove memcmp calls non-compliant with Rule 21.16.
On 6/5/25 02:31, Jan Beulich wrote: > On 05.06.2025 01:35, Stefano Stabellini wrote: >> From: Alessandro Zucchelli <alessandro.zucchelli@xxxxxxxxxxx> >> >> 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.zucchelli@xxxxxxxxxxx> > > 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) Attachment:
OpenPGP_0xB288B55FFF9C22C1.asc Attachment:
OpenPGP_signature.asc
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |