> On 31 Oct 2023, at 15:36, Julien Grall <jul...@xen.org> wrote:
>
>
>
> On 31/10/2023 15:32, Luca Fancellu wrote:
>>> On 31 Oct 2023, at 15:27, Julien Grall <jul...@xen.org> wrote:
>>>
>>> Hi,
>>>
>>> On 31/10/2023 15:12, Luca Fancellu wrote:
>>>>> On 31 Oct 2023, at 15:10, Nicola Vetrini <nicola.vetr...@bugseng.com>
>>>>> wrote:
>>>>>
>>>>> On 2023-10-31 15:13, Luca Fancellu wrote:
>>>>>>> On 31 Oct 2023, at 13:27, Julien Grall <jul...@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.
>>>> Yeah, but we could rule out something in code_style to allow only this
>>>> kind of trailing comments to exceed the 80 chars
>>>
>>> In the past I expressed concerned with this kind of the rule because it is
>>> not entirely clear how an automatic formatter will be able to check it.
>>>
>>> Can you clarify whether clang-format would be able to handle your proposed
>>> rule?
>> So, yesterday Bertrand pointed out a StackOverflow thread for this issue and
>> if we use ReflowComments: false we should
>> be able to let the line as it is (not tested).
>
> Wouldn't that prevent reflow for all the comments? If so, I don't think this
> is we want. Instead, we want to allow reflow for any comments but the one
> done at the end of the line.
Ok well, I was optimistic, in reality with the option as false, it would anyway
reflow the line leaving the comment untouched.
E.g. from this:
if ( modrm_mod == MASK_EXTR(instr_modrm, 0300) && /* SAF-2-safe
octal-ok */
(modrm_reg & 7) == MASK_EXTR(instr_modrm, 0070) && /* SAF-2-safe
octal-ok */
(modrm_rm & 7) == MASK_EXTR(instr_modrm, 0007) ) /* SAF-2-safe
octal-ok */
return emul_len;
To this:
if ( modrm_mod ==
MASK_EXTR(instr_modrm, 0300) && /* SAF-2-safe octal-ok */
(modrm_reg & 7) ==
MASK_EXTR(instr_modrm, 0070) && /* SAF-2-safe octal-ok */
(modrm_rm & 7) ==
MASK_EXTR(instr_modrm, 0007) ) /* SAF-2-safe octal-ok */
return emul_len;
... sigh...
