Re: MISRA: Compatible declarations for sort and bsearch

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

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)