Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1

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

On 2023-10-31 15:13, Luca Fancellu wrote:
>> On 31 Oct 2023, at 13:27, Julien Grall <julien@xen.org> wrote:
>> 
>> Hi Stefano,
>> 
>> On 30/10/2023 22:49, Stefano Stabellini wrote:
>>> On Mon, 30 Oct 2023, Julien Grall wrote:
>>>> Hi Nicola,
>>>> 
>>>> On 27/10/2023 16:11, Nicola Vetrini wrote:
>>>>> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
>>>>> index 8511a189253b..8aaaa1473fb4 100644
>>>>> --- a/docs/misra/deviations.rst
>>>>> +++ b/docs/misra/deviations.rst
>>>>> @@ -90,6 +90,13 @@ Deviations related to MISRA C:2012 Rules:
>>>>>            - __emulate_2op and __emulate_2op_nobyte
>>>>>            - read_debugreg and write_debugreg
>>>>>   +   * - R7.1
>>>>> +     - It is safe to use certain octal constants the way they are 
>>>>> defined
>>>>> +       in specifications, manuals, and algorithm descriptions. 
>>>>> Such places
>>>>> +       are marked safe with a /\* octal-ok \*/ in-code comment, or 
>>>>> with a
>>>>> SAF
>>>>> +       comment (see safe.json).
>>>> 
>>>> Reading this, it is unclear to me why we have two ways to deviate 
>>>> the rule
>>>> r7.1. And more importantely, how would the developper decide which 
>>>> one to use?
>>> I agree with you on this and we were discussing this topic just this
>>> morning in the FUSA community call. I think we need a way to do this
>>> with the SAF framework:
>>> if (some code with violation) /* SAF-xx-safe */
>>> This doesn't work today unfortunately. It can only be done this way:
>>> /* SAF-xx-safe */
>>> if (some code with violation)
>>> Which is not always desirable. octal-ok is just an ad-hoc solution 
>>> for
>>> one specific violation but we need a generic way to do this. Luca is
>>> investigating possible ways to support the previous format in SAF.
>> 
>> Why can't we use octal-ok everywhere for now? My point here is to make 
>> simple for the developper to know what to use.
>> 
>>> I think we should take this patch for now and harmonize it once SAF 
>>> is
>>> improved.
>> 
>> The description of the deviation needs some improvement. To give an 
>> example, with the current wording, one could they can use octal-ok 
>> everywhere. But above, you are implying that SAF-xx-safe should be
>> preferred.
>> 
>> I would still strongly prefer if we use octal-ok everywhere because 
>> this is simple to remember. But if the other are happy to have both 
>> SAF-XX and octal-ok, then the description needs to be completely 
>> unambiguous and the patch should contain some explanation why we have 
>> two different ways to deviate.
> 
> Would it be ok to have both, for example: /* SAF-XX-safe octal-ok */
> 
> So that the suppression engine do what it should (currently it doesn’t 
> suppress the same line, but we could do something about it) and the 
> developer
> has a way to understand what is the violation here without going to the 
> justification database.

I guess. It could overflow the 80-char limit in 
xen/arch/x86/hvm/svm/svm.h, though.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)