Re: [XEN PATCH v3 01/16] misra: add deviation for headers that explicitly avoid guards

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 15 Mar 2024, Jan Beulich wrote:
> On 14.03.2024 23:55, Stefano Stabellini wrote:
> > On Mon, 11 Mar 2024, Jan Beulich wrote:
> >> On 11.03.2024 09:59, Simone Ballarin wrote:
> >>> Some headers, under specific circumstances (documented in a comment at
> >>> the beginning of the file), explicitly avoid inclusion guards: the caller
> >>> is responsible for including them correctly.
> >>>
> >>> These files are not supposed to comply with Directive 4.10:
> >>> "Precautions shall be taken in order to prevent the contents of a header
> >>> file being included more than once"
> >>>
> >>> This patch adds deviation cooments for headers that avoid guards.
> >>>
> >>> Signed-off-by: Simone Ballarin <simone.ballarin@xxxxxxxxxxx>
> >>>
> >>> ---
> >>> Changes in v3:
> >>> - fix inconsistent deviation ID
> >>> - change comment-based deviation text
> >>> Changes in v2:
> >>> - use the format introduced with doc/misra/safe.json instead of
> >>>   a generic text-based deviation
> >>> ---
> >>>  docs/misra/safe.json                        | 9 +++++++++
> >>>  xen/include/public/arch-x86/cpufeatureset.h | 1 +
> >>>  xen/include/public/errno.h                  | 1 +
> >>>  3 files changed, 11 insertions(+)
> >>
> >> I understand something wants doing, but having such comments appear in 
> >> public
> >> headers feels wrong to me. I'm afraid I have no good alternative 
> >> suggestion.
> > 
> > Given that in both cases there is very nice explanation on how to use
> > the headers as an in-code comment just above, could we embed the
> > SAF-3-safe marker in the existing comment?
> 
> I'm afraid that won't address my remark, and I'm further afraid this would
> then render the SAF part of the comment ineffectual.
> 
> > If not, I think we should go with this patch as is (I don't think it is
> > worth my, your, and Simone's time to look for alternatives).
> 
> Easy alternative: Simply leave public headers alone.

I don't think it is a good idea to skip MISRA on public headers as they
are used as part of the Xen build. I think adding the one line SAF
comment is better.

We still have strange tags like the following in public headers:

 * `incontents 150 evtchn Event Channels

What does `incontents 150 even mean? If I grep for "incontents" I cannot
find any explanation or definition. The SAF comment is easily greppable
and with a clear definition. I am in favor of adding them (and removing
"incontents" but that's a discussion for another day.)