On 06.06.2025 22:49, Stefano Stabellini wrote: > On Fri, 6 Jun 2025, Nicola Vetrini wrote: >>>>> 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? >>> >>> 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? >>> >> >> While I agree that the result of the comparison is correct either way in >> these >> cases, the rule is written to be simple to apply (i.e., not limited only to >> those cases that may differ), and in particular in the rationale it is >> indicated that using memcmp to compare string *may* indicate a mistake. As >> written above, deviating the string literal comparisons is an option, which >> can be justified with efficiency concerns, but it goes a bit against the >> rationale of the rule itself. > > Also looking at Andrew's reply, it seems that the preference is to > deviate string literals. The change to docs/misra/rules.rst is easy > enough, but I am not sure how to make the corresponding change to > analysis.ecl. > > diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst > index e1c26030e8..56b6e351df 100644 > --- a/docs/misra/rules.rst > +++ b/docs/misra/rules.rst > @@ -813,7 +813,7 @@ maintainers if you want to suggest a change. > shall point to either a pointer type, an essentially signed type, > an essentially unsigned type, an essentially Boolean type or an > essentially enum type > - - void* arguments are allowed > + - void* and string literals arguments are allowed
Yet as per my earlier reply: This would go too far, wouldn't it? Jan
