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

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

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@kernel.org>
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> 
> 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)