Re: [XEN PATCH][for-4.19 v2 2/2] docs/misra: add deviations.rst to document additional deviations.

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 10/10/2023 03:19, Stefano Stabellini wrote:
> +Henry
>
> On Mon, 9 Oct 2023, Nicola Vetrini wrote:
> > This file contains the deviation that are not marked by
> > a deviation comment, as specified in
> > docs/misra/documenting-violations.rst.
> >
> > Suggested-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> This is great! Thank you so much!
>
> I have a few questions below but even as-is it is way better than
> nothing. I think we should add this for 4.18
>
>
> > ---
> >  docs/index.rst            |   1 +
> >  docs/misra/deviations.rst | 240 
> > ++++++++++++++++++++++++++++++++++++++
> >  docs/misra/rules.rst      |   2 +-
> >  3 files changed, 242 insertions(+), 1 deletion(-)
> >  create mode 100644 docs/misra/deviations.rst
> >
> > diff --git a/docs/index.rst b/docs/index.rst
> > index 2c47cfa999f2..f3f779f89ce5 100644
> > --- a/docs/index.rst
> > +++ b/docs/index.rst
> > @@ -63,6 +63,7 @@ Xen hypervisor code.
> >     :maxdepth: 2
> >
> >     misra/rules
> > +   misra/deviations
> >
> >
> >  Miscellanea
> > diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> > new file mode 100644
> > index 000000000000..19743e34ce03
> > --- /dev/null
> > +++ b/docs/misra/deviations.rst
> > @@ -0,0 +1,240 @@
> > +.. SPDX-License-Identifier: CC-BY-4.0
> > +
> > +MISRA C deviations for Xen
> > +==========================
> > +
> > +The following is the list of MISRA C:2012 deviations for the Xen 
> > codebase that
> > +are not covered by a `SAF-x-safe` or `SAF-x-false-positive-<tool>` 
> > comment, as
> > +specified in docs/misra/documenting-violations.rst; the lack of
> > +such comments is usually due to the excessive clutter they would 
> > bring to the
> > +codebase or the impossibility to express such a deviation (e.g., if 
> > it's
> > +composed of several conditions).
> > +
> > +Deviations related to MISRA C:2012 Directives:
> > +----------------------------------------------
> > +
> > +.. list-table::
> > +   :header-rows: 1
> > +
> > +   * - Directive identifier
> > +     - Justification
> > +     - Notes
> > +
> > +   * - D4.3
> > +     - Accepted for the ARM64 codebase
> > +     - Tagged as `disapplied` for ECLAIR on any other violation 
> > report.
> This mean it has been applied for ARM64 but not x86, right?
Yes.

> > +   * - D4.3
> > +     - The inline asm in 'xen/arch/arm/arm64/lib/bitops.c' is tightly 
> > coupled
> > +       with the surronding C code that acts as a wrapper, so it has 
> > been decided
> > +       not to add an additional encapsulation layer.
> > +     - Tagged as `deliberate` for ECLAIR.
> > +
> > +Deviations related to MISRA C:2012 Rules:
> > +-----------------------------------------
> > +
> > +.. list-table::
> > +   :header-rows: 1
> > +
> > +   * - Rule identifier
> > +     - Justification
> > +     - Notes
> > +
> > +   * - R2.1
> > +     - The compiler implementation guarantees that the unreachable 
> > code is
> > +       removed. Constant expressions and unreachable branches of if 
> > and switch
> > +       statements are expected.
> > +     - Tagged as `safe` for ECLAIR.
> > +
> > +   * - R2.1
> > +     - Some functions are intended not to be referenced.
> > +     - Tagged as `deliberate` for ECLAIR.
> What does it mean "some functions" in this case? Should we list which
> functions?
Well, there are a lot, typically resulting from build configurations 
that do not
use them, or because they are used only in asm code. I can mention these 
reasons in the
document, to make it easier to understand.

> Other than this, I checked and everything else looks great
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)