[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 Fri, 6 Jun 2025, Nicola Vetrini wrote:
> > > > 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?
> > 
> 
> 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
 
    * - `Rule 21.17 
<https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_21_17.c>`_
      - Mandatory



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.