Re: [XEN PATCH 05/13] automation/eclair: add deviation for usercopy.c

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

> On 28 Aug 2023, at 23:27, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> 
> +Nicola, Luca
> 
> On Mon, 28 Aug 2023, Simone Ballarin wrote:
>> xen/arch/x86/usercopy.c includes itself, so it is 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 a deviation for the file.
>> 
>> Signed-off-by: Simone Ballarin <simone.ballarin@xxxxxxxxxxx>
>> 
>> ---
>> automation/eclair_analysis/ECLAIR/deviations.ecl | 4 ++++
>> docs/misra/rules.rst                             | 2 ++
>> 2 files changed, 6 insertions(+)
>> 
>> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
>> b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> index 2681a4cff5..a7d4f29b43 100644
>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> @@ -96,6 +96,10 @@ conform to the directive."
>> -config=MC3R1.D4.10,reports+={safe, "first_area(text(^ \\* In this case, no 
>> inclusion guards apply and the caller is responsible.*\\*/$, begin-1))"}
>> -doc_end
>> 
>> +-doc_begin="xen/arch/x86/usercopy.c includes itself: it is not supposed to 
>> comply with the directive"
>> +-config=MC3R1.D4.10,reports+={deliberate, 
>> "all_area(all_loc(file("^xen/arch/x86/usercopy\\.c$")))"}
>> +-doc_end
>> +
>> #
>> # Series 5.
>> #
>> diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
>> index 4b1a7b02b6..45e13d0302 100644
>> --- a/docs/misra/rules.rst
>> +++ b/docs/misra/rules.rst
>> @@ -62,6 +62,8 @@ maintainers if you want to suggest a change.
>>      - Files that are intended to be included more than once do not need to
>>        conform to the directive. Files that explicitly avoid inclusion guards
>>        under specific circumstances do not need to conform the directive.
>> +       xen/arch/x86/usercopy.c includes itself: it is not supposed to comply
>> +       with the directive.
> 
> 
> We need to find a consistent way to document this kind of deviations in
> a non-ECLAIR specific way, without adding the complete list of
> deviations to rules.rst.
> 
> Can we use safe.json and add an in-code comment at the top of
> usercopy.c? E.g.:
> 
> diff --git a/xen/arch/x86/usercopy.c b/xen/arch/x86/usercopy.c
> index b8c2d1cc0b..8bb591f472 100644
> --- a/xen/arch/x86/usercopy.c
> +++ b/xen/arch/x86/usercopy.c
> @@ -1,3 +1,4 @@
> +/* SAF-1-safe */
> /* 
>  * User address space access functions.
>  *
> 
> Otherwise, maybe we should extend safe.json to also have an extra field
> with a list of paths. For instance see "files" below:
> 
> {
>    "version": "1.0",
>    "content": [
>        {
>            "id": "SAF-0-safe",
>            "analyser": {
>                "eclair": "MC3R1.R8.6",
>                "coverity": "misra_c_2012_rule_8_6_violation"
>            },
>            "name": "Rule 8.6: linker script defined symbols",
>            "text": "It is safe to declare this symbol because it is defined 
> in the linker script."
>        },
>        {
>            "id": "SAF-1-safe",
>            "analyser": {
>                "eclair": "MC3R1.D4.10"
>            },
>            "name": "Dir 4.10: files that include themselves",
>            "text": "Files purposely written to include themselves are not 
> supposed to comply with D4.10.",
>            "files": ["xen/arch/x86/usercopy.c"]

Why couldn’t we do it without the “files” field? The presence of the tag in the 
file and the justification (I think)
are enough. 

>        },
>        {
>            "id": "SAF-2-safe",
>            "analyser": {},
>            "name": "Sentinel",
>            "text": "Next ID to be used"
>        }
>    ]
> }