Re: MISRA: Compatible declarations for sort and bsearch

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2023-11-27 15:59, Jan Beulich wrote:
> On 27.11.2023 15:32, Nicola Vetrini wrote:
> > Still on the matter of Rule 8.4, though not related to bsearch or 
> > sort:
> > - the definition of do_mca in x86/cpu/mcheck/mca.c has the following
> > header:
> >    #include <xen/hypercall.h> /* for do_mca */
> >    which in turn leads to x86/include/asm/hypercall.h, which includes 
> > the
> > following:
> >    #include <public/arch-x86/xen-mca.h> /* for do_mca */
> >
> >    where I can't see a declaration for do_mca, as I would have 
> > expected.
> > I'd like to understand what's going on here, since I may be missing 
> > some
> > piece of information (perhaps something is generated during the 
> > build).
> It can't possibly live in the public header. The comment simply went
> stale with the auto-generation of headers; the decl is in 
> hypercall-defs.h
> now.
Ok, thanks.

> > - x86/traps.c do_general_protection may want a declaration in
> > x86/include/asm/traps.h, or perhaps it should gain the asmlinkage
> > attribute, given that it's used only by asm and the TU that defines 
> > it.
> Neither is really attractive imo.
>
> > - function test and variable data in x86/efi/check.c look like they
> > should not be MISRA compliant, so they may be added to the
> > exclude-list.json
> This file isn't contributing to the final binary.
Then I'll exclude them

> > - given the comment in xen/common/page_alloc.c for first_valid_mfn
> >
> > /*
> >   * first_valid_mfn is exported because it is use in ARM specific NUMA
> >   * helpers. See comment in arch/arm/include/asm/numa.h.
> >   */
> > mfn_t first_valid_mfn = INVALID_MFN_INITIALIZER;
> >
> > and the related ARM comment
> >
> > /*
> >   * TODO: make first_valid_mfn static when NUMA is supported on Arm, 
> > this
> >   * is required because the dummy helpers are using it.
> >   */
> > extern mfn_t first_valid_mfn;
> >
> > it should probably be deviated.
> NUMA work is still in progress for Arm, I think, so I'd rather wait 
> with
> deviating.
+Stefano

I can leave it as is, if that's indeed going to become static at some 
point.
> > - compat_set_{px,cx}_pminfo in x86/x86_64/cpufreq.c are perhaps 
> > declared
> > with an autogenerated header?
> I don't think so. Only top-level hypercall handlers would be. This 
> works by
> (perhaps even unintentional) trickery: xen/pmstat.h is included only 
> after
> set_{c,p}x_pminfo are re-defined to compat_set_{c,p}x_pminfo, so the 
> same
> declarations happen to serve two purposes (but of course don't provide 
> the
> intended caller/callee agreement).
I didn't understand your explanation fully; I see xen/pmstat.h in 
cpufreq.c included before
compat/platform.h which, as I understand it, redefines set_{c,p}x_pminfo 
to compat_set_{c,p}x_pminfo, therefore down below no declaration for 
compat_set_{c,p}x_pminfo is visible, triggering the violation.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)