Re: [XEN PATCH v3 03/16] misra: add deviations for direct inclusion guards

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.03.2024 23:59, Stefano Stabellini wrote:
> On Mon, 11 Mar 2024, Simone Ballarin wrote:
>> On 11/03/24 14:56, Jan Beulich wrote:
>>> On 11.03.2024 13:00, Simone Ballarin wrote:
>>>> On 11/03/24 11:08, Jan Beulich wrote:
>>>>> On 11.03.2024 09:59, Simone Ballarin wrote:
>>>>>> --- a/xen/arch/arm/include/asm/hypercall.h
>>>>>> +++ b/xen/arch/arm/include/asm/hypercall.h
>>>>>> @@ -1,3 +1,4 @@
>>>>>> +/* SAF-5-safe direct inclusion guard before */
>>>>>>    #ifndef __XEN_HYPERCALL_H__
>>>>>>    #error "asm/hypercall.h should not be included directly - include
>>>>>> xen/hypercall.h instead"
>>>>>>    #endif
>>>>>> --- a/xen/arch/x86/include/asm/hypercall.h
>>>>>> +++ b/xen/arch/x86/include/asm/hypercall.h
>>>>>> @@ -2,6 +2,7 @@
>>>>>>     * asm-x86/hypercall.h
>>>>>>     */
>>>>>>    +/* SAF-5-safe direct inclusion guard before */
>>>>>>    #ifndef __XEN_HYPERCALL_H__
>>>>>>    #error "asm/hypercall.h should not be included directly - include
>>>>>> xen/hypercall.h instead"
>>>>>>    #endif
>>>>>
>>>>> Iirc it was said that this way checking for correct guards is suppressed
>>>>> altogether in Eclair, which is not what we want. Can you clarify this,
>>>>> please?
>>>>>
>>>>
>>>> My first change was moving this check inside the guard.
>>>> You commented my patch saying that this would be an error because someone
>>>> can
>>>> include it directly if it has already been included indirectly.
>>>> I replied telling that this was the case also before the change.
>>>> You agreed with me, and we decided that the correct thing would be fixing
>>>> the
>>>> check and not apply my temporary change to address the finding.
>>>>
>>>> Considering that the code should be amended, a SAF deviation seems to me
>>>> the most appropriate way for suppressing these findings.
>>>
>>> Since I don't feel your reply addresses my question, asking differently:
>>> With
>>> your change in place, will failure to have proper guards (later) in these
>>> headers still be reported by Eclair?
>>
>> No, if you put something between the check and the guard,
>> no violation will be reported.
> 
> From this email exchange I cannot under if Jan is OK with this patch or
> not.

Whether I'm okay(ish) with the patch here depends on our position towards
the lost checking in Eclair mentioned above. To me it still looks relevant
that checking for a guard occurs, even if that isn't first in a file for
some specific reason.

Jan