[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 Thu, 5 Jun 2025, 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? I'll add the S-o-B and fix the 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.) If we consider string literals "pointer types", then I think you are right that this would fall under what is permitted by 21.16. Nicola, what do you think? > > @@ -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). The adjustments can 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. Yes
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |