* Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1 [not found] <0c86b50a796ba2aaab01e30935ed559ab719d33a.1698418578.git.nicola.vetrini@bugseng.com> @ 2023-10-27 20:52 ` Stefano Stabellini 2023-10-30 14:51 ` Julien Grall 1 sibling, 0 replies; 13+ messages in thread From: Stefano Stabellini @ 2023-10-27 20:52 UTC (permalink / raw) To: Nicola Vetrini Cc: xen-devel, sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder, consulting, jbeulich, andrew.cooper3, roger.pau, Simone Ballarin, Doug Goldstein, George Dunlap, Julien Grall, Wei Liu On Fri, 27 Oct 2023, Nicola Vetrini wrote: > As specified in rules.rst, these constants can be used > in the code. > > Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1 [not found] <0c86b50a796ba2aaab01e30935ed559ab719d33a.1698418578.git.nicola.vetrini@bugseng.com> 2023-10-27 20:52 ` [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1 Stefano Stabellini @ 2023-10-30 14:51 ` Julien Grall 2023-10-30 22:49 ` Stefano Stabellini 1 sibling, 1 reply; 13+ messages in thread From: Julien Grall @ 2023-10-30 14:51 UTC (permalink / raw) To: Nicola Vetrini, xen-devel Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder, consulting, jbeulich, andrew.cooper3, roger.pau, Simone Ballarin, Doug Goldstein, George Dunlap, Wei Liu 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? Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1 2023-10-30 14:51 ` Julien Grall @ 2023-10-30 22:49 ` Stefano Stabellini 2023-10-31 13:27 ` Julien Grall 0 siblings, 1 reply; 13+ messages in thread From: Stefano Stabellini @ 2023-10-30 22:49 UTC (permalink / raw) To: Julien Grall Cc: Nicola Vetrini, xen-devel, sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder, consulting, jbeulich, andrew.cooper3, roger.pau, Simone Ballarin, Doug Goldstein, George Dunlap, Wei Liu 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. I think we should take this patch for now and harmonize it once SAF is improved. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1 2023-10-30 22:49 ` Stefano Stabellini @ 2023-10-31 13:27 ` Julien Grall 2023-10-31 14:13 ` Luca Fancellu 2023-10-31 21:41 ` Stefano Stabellini 0 siblings, 2 replies; 13+ messages in thread From: Julien Grall @ 2023-10-31 13:27 UTC (permalink / raw) To: Stefano Stabellini Cc: Nicola Vetrini, xen-devel, michal.orzel, xenia.ragiadakou, ayan.kumar.halder, consulting, jbeulich, andrew.cooper3, roger.pau, Simone Ballarin, Doug Goldstein, George Dunlap, Wei Liu 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. Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1 2023-10-31 13:27 ` Julien Grall @ 2023-10-31 14:13 ` Luca Fancellu 2023-10-31 15:10 ` Nicola Vetrini 2023-10-31 21:41 ` Stefano Stabellini 1 sibling, 1 reply; 13+ messages in thread From: Luca Fancellu @ 2023-10-31 14:13 UTC (permalink / raw) To: Julien Grall Cc: Stefano Stabellini, Nicola Vetrini, Xen-devel, michal.orzel@amd.com, xenia.ragiadakou@amd.com, ayan.kumar.halder@amd.com, consulting@bugseng.com, jbeulich@suse.com, andrew.cooper3@citrix.com, roger.pau@citrix.com, Simone Ballarin, Doug Goldstein, George Dunlap, Wei Liu > 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1 2023-10-31 14:13 ` Luca Fancellu @ 2023-10-31 15:10 ` Nicola Vetrini 2023-10-31 15:12 ` Luca Fancellu 0 siblings, 1 reply; 13+ messages in thread From: Nicola Vetrini @ 2023-10-31 15:10 UTC (permalink / raw) To: Luca Fancellu Cc: Julien Grall, Stefano Stabellini, Xen-devel, michal.orzel, xenia.ragiadakou, ayan.kumar.halder, consulting, jbeulich, andrew.cooper3, roger.pau, Simone Ballarin, Doug Goldstein, George Dunlap, Wei Liu 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) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1 2023-10-31 15:10 ` Nicola Vetrini @ 2023-10-31 15:12 ` Luca Fancellu 2023-10-31 15:27 ` Julien Grall 0 siblings, 1 reply; 13+ messages in thread From: Luca Fancellu @ 2023-10-31 15:12 UTC (permalink / raw) To: Nicola Vetrini Cc: Julien Grall, Stefano Stabellini, Xen-devel, michal.orzel@amd.com, xenia.ragiadakou@amd.com, ayan.kumar.halder@amd.com, consulting@bugseng.com, jbeulich@suse.com, andrew.cooper3@citrix.com, roger.pau@citrix.com, Simone Ballarin, Doug Goldstein, George Dunlap, Wei Liu > On 31 Oct 2023, at 15:10, Nicola Vetrini <nicola.vetrini@bugseng.com> wrote: > > 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. Yeah, but we could rule out something in code_style to allow only this kind of trailing comments to exceed the 80 chars > > -- > Nicola Vetrini, BSc > Software Engineer, BUGSENG srl (https://bugseng.com) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1 2023-10-31 15:12 ` Luca Fancellu @ 2023-10-31 15:27 ` Julien Grall 2023-10-31 15:32 ` Luca Fancellu 0 siblings, 1 reply; 13+ messages in thread From: Julien Grall @ 2023-10-31 15:27 UTC (permalink / raw) To: Luca Fancellu, Nicola Vetrini Cc: Stefano Stabellini, Xen-devel, michal.orzel@amd.com, xenia.ragiadakou@amd.com, ayan.kumar.halder@amd.com, consulting@bugseng.com, jbeulich@suse.com, andrew.cooper3@citrix.com, roger.pau@citrix.com, Simone Ballarin, Doug Goldstein, George Dunlap, Wei Liu Hi, On 31/10/2023 15:12, Luca Fancellu wrote: >> On 31 Oct 2023, at 15:10, Nicola Vetrini <nicola.vetrini@bugseng.com> wrote: >> >> 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. > > 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? Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1 2023-10-31 15:27 ` Julien Grall @ 2023-10-31 15:32 ` Luca Fancellu 2023-10-31 15:36 ` Julien Grall 0 siblings, 1 reply; 13+ messages in thread From: Luca Fancellu @ 2023-10-31 15:32 UTC (permalink / raw) To: Julien Grall Cc: Nicola Vetrini, Stefano Stabellini, Xen-devel, michal.orzel@amd.com, xenia.ragiadakou@amd.com, ayan.kumar.halder@amd.com, consulting@bugseng.com, jbeulich@suse.com, andrew.cooper3@citrix.com, roger.pau@citrix.com, Simone Ballarin, Doug Goldstein, George Dunlap, Wei Liu > On 31 Oct 2023, at 15:27, Julien Grall <julien@xen.org> wrote: > > Hi, > > On 31/10/2023 15:12, Luca Fancellu wrote: >>> On 31 Oct 2023, at 15:10, Nicola Vetrini <nicola.vetrini@bugseng.com> wrote: >>> >>> 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. >> 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). https://clang.llvm.org/docs/ClangFormatStyleOptions.html#reflowcomments > > Cheers, > > -- > Julien Grall ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1 2023-10-31 15:32 ` Luca Fancellu @ 2023-10-31 15:36 ` Julien Grall 2023-10-31 16:09 ` Luca Fancellu 0 siblings, 1 reply; 13+ messages in thread From: Julien Grall @ 2023-10-31 15:36 UTC (permalink / raw) To: Luca Fancellu Cc: Nicola Vetrini, Stefano Stabellini, Xen-devel, michal.orzel@amd.com, xenia.ragiadakou@amd.com, ayan.kumar.halder@amd.com, consulting@bugseng.com, jbeulich@suse.com, andrew.cooper3@citrix.com, roger.pau@citrix.com, Simone Ballarin, Doug Goldstein, George Dunlap, Wei Liu On 31/10/2023 15:32, Luca Fancellu wrote: > > >> On 31 Oct 2023, at 15:27, Julien Grall <julien@xen.org> wrote: >> >> Hi, >> >> On 31/10/2023 15:12, Luca Fancellu wrote: >>>> On 31 Oct 2023, at 15:10, Nicola Vetrini <nicola.vetrini@bugseng.com> wrote: >>>> >>>> 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. >>> 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. Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1 2023-10-31 15:36 ` Julien Grall @ 2023-10-31 16:09 ` Luca Fancellu 0 siblings, 0 replies; 13+ messages in thread From: Luca Fancellu @ 2023-10-31 16:09 UTC (permalink / raw) To: Julien Grall Cc: Nicola Vetrini, Stefano Stabellini, Xen-devel, michal.orzel@amd.com, xenia.ragiadakou@amd.com, ayan.kumar.halder@amd.com, consulting@bugseng.com, jbeulich@suse.com, andrew.cooper3@citrix.com, roger.pau@citrix.com, Simone Ballarin, Doug Goldstein, George Dunlap, Wei Liu > On 31 Oct 2023, at 15:36, Julien Grall <julien@xen.org> wrote: > > > > On 31/10/2023 15:32, Luca Fancellu wrote: >>> On 31 Oct 2023, at 15:27, Julien Grall <julien@xen.org> wrote: >>> >>> Hi, >>> >>> On 31/10/2023 15:12, Luca Fancellu wrote: >>>>> On 31 Oct 2023, at 15:10, Nicola Vetrini <nicola.vetrini@bugseng.com> wrote: >>>>> >>>>> 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. >>>> 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... ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1 2023-10-31 13:27 ` Julien Grall 2023-10-31 14:13 ` Luca Fancellu @ 2023-10-31 21:41 ` Stefano Stabellini 2023-11-02 8:36 ` Nicola Vetrini 1 sibling, 1 reply; 13+ messages in thread From: Stefano Stabellini @ 2023-10-31 21:41 UTC (permalink / raw) To: Julien Grall Cc: Stefano Stabellini, Nicola Vetrini, xen-devel, michal.orzel, xenia.ragiadakou, ayan.kumar.halder, consulting, jbeulich, andrew.cooper3, roger.pau, Simone Ballarin, Doug Goldstein, George Dunlap, Wei Liu On Tue, 30 Oct 2023, Julien Grall 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? I think this is a good option for now, yes > 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. +1 > 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. I think we could say "octal-ok" only and not mention SAF. As you can see from the other messages we still have work to do on SAF to be able to use it the way we would like to use it. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1 2023-10-31 21:41 ` Stefano Stabellini @ 2023-11-02 8:36 ` Nicola Vetrini 0 siblings, 0 replies; 13+ messages in thread From: Nicola Vetrini @ 2023-11-02 8:36 UTC (permalink / raw) To: Stefano Stabellini, Julien Grall Cc: Julien Grall, xen-devel, michal.orzel, xenia.ragiadakou, ayan.kumar.halder, consulting, jbeulich, andrew.cooper3, roger.pau, Simone Ballarin, Doug Goldstein, George Dunlap, Wei Liu Hi Julien, Stefano On 2023-10-31 22:41, Stefano Stabellini wrote: > On Tue, 30 Oct 2023, Julien Grall 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? > > I think this is a good option for now, yes > > >> 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. > > +1 > > >> 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. > > I think we could say "octal-ok" only and not mention SAF. As you can > see > from the other messages we still have work to do on SAF to be able to > use it the way we would like to use it. Thanks for the feedback; I'll revise the patch to use and mention only octal-ok. -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com) ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-11-02 8:36 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <0c86b50a796ba2aaab01e30935ed559ab719d33a.1698418578.git.nicola.vetrini@bugseng.com>
2023-10-27 20:52 ` [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1 Stefano Stabellini
2023-10-30 14:51 ` Julien Grall
2023-10-30 22:49 ` Stefano Stabellini
2023-10-31 13:27 ` Julien Grall
2023-10-31 14:13 ` Luca Fancellu
2023-10-31 15:10 ` Nicola Vetrini
2023-10-31 15:12 ` Luca Fancellu
2023-10-31 15:27 ` Julien Grall
2023-10-31 15:32 ` Luca Fancellu
2023-10-31 15:36 ` Julien Grall
2023-10-31 16:09 ` Luca Fancellu
2023-10-31 21:41 ` Stefano Stabellini
2023-11-02 8:36 ` Nicola Vetrini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.