1. Patchew
  2. Xen
  3. View series

[PATCH v2] docs/misra/rules.rst: add Rule 17.1

Stefano Stabellini posted 1 patch 4 months, 4 weeks ago
Failed in applying to current master (apply log)
[PATCH v2] docs/misra/rules.rst: add Rule 17.1
Posted by Stefano Stabellini 4 months, 4 weeks ago 

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
Changes in v2:
- separated 17.1 in its own patch
- add a comment

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 8a659d8d47..f29b4c3d9a 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -189,6 +189,12 @@ existing codebase are work-in-progress.
      - A switch-expression shall not have essentially Boolean type
      -
 
+   * - `Rule 17.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_17_01.c>`_
+     - Required
+     - The features of <stdarg.h> shall not be used
+     - It is understood that in some limited circumstances <stdarg.h> is
+       appropriate to use, such as the implementation of printk.
+
    * - `Rule 17.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_17_03.c>`_
      - Mandatory
      - A function shall not be declared implicitly
Re: [PATCH v2] docs/misra/rules.rst: add Rule 17.1
Posted by Julien Grall 4 months, 3 weeks ago 
Hi Stefano,

On 09/12/2023 01:39, Stefano Stabellini wrote:
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> ---
> Changes in v2:
> - separated 17.1 in its own patch
> - add a comment
> 
> diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
> index 8a659d8d47..f29b4c3d9a 100644
> --- a/docs/misra/rules.rst
> +++ b/docs/misra/rules.rst
> @@ -189,6 +189,12 @@ existing codebase are work-in-progress.
>        - A switch-expression shall not have essentially Boolean type
>        -
>   
> +   * - `Rule 17.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_17_01.c>`_
> +     - Required
> +     - The features of <stdarg.h> shall not be used
> +     - It is understood that in some limited circumstances <stdarg.h> is
> +       appropriate to use, such as the implementation of printk.

The last bullet point is unclear to me. You don't define what 
"appropriate" means here. So who is going to decide? Also, how is this 
going to be deviated?

Possibly the solution here is to remove the last bullet point and have a 
paragraph in deviations.rst explaining why we are using va_args.

Cheers,

-- 
Julien Grall
Re: [PATCH v2] docs/misra/rules.rst: add Rule 17.1
Posted by Stefano Stabellini 4 months, 3 weeks ago 
On Fri, 15 Dec 2023, Julien Grall wrote:
> Hi Stefano,
> 
> On 09/12/2023 01:39, Stefano Stabellini wrote:
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> > ---
> > Changes in v2:
> > - separated 17.1 in its own patch
> > - add a comment
> > 
> > diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
> > index 8a659d8d47..f29b4c3d9a 100644
> > --- a/docs/misra/rules.rst
> > +++ b/docs/misra/rules.rst
> > @@ -189,6 +189,12 @@ existing codebase are work-in-progress.
> >        - A switch-expression shall not have essentially Boolean type
> >        -
> >   +   * - `Rule 17.1
> > <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_17_01.c>`_
> > +     - Required
> > +     - The features of <stdarg.h> shall not be used
> > +     - It is understood that in some limited circumstances <stdarg.h> is
> > +       appropriate to use, such as the implementation of printk.
> 
> The last bullet point is unclear to me. You don't define what "appropriate"
> means here. So who is going to decide? Also, how is this going to be deviated?
> 
> Possibly the solution here is to remove the last bullet point and have a
> paragraph in deviations.rst explaining why we are using va_args.

Actually, I agree with you. I added the last bullet to address Jan's
concern:
https://marc.info/?l=xen-devel&m=170191695511513
https://marc.info/?l=xen-devel&m=170193528120968

This was my original reply:

"We agreed that in certain situations stdarg.h is OK to use and in those
cases we would add a deviation. Would you like me to add something to
that effect here? I could do that but it would sound a bit vague.  Also
if we want to specify a project-wide deviation it would be better
documented in docs/misra/deviations.rst. I would leave Rule 17.1 without
a note."

My preference is still to remove the last bullet (because too generic)
and add any specific information to deviations.rst as usual.

Julien, would you be OK with this patch if I remove the last bullet and
leave it blank?
Re: [PATCH v2] docs/misra/rules.rst: add Rule 17.1
Posted by Julien Grall 4 months, 2 weeks ago 
Hi Stefano,

On 15/12/2023 21:02, Stefano Stabellini wrote:
> On Fri, 15 Dec 2023, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 09/12/2023 01:39, Stefano Stabellini wrote:
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>> ---
>>> Changes in v2:
>>> - separated 17.1 in its own patch
>>> - add a comment
>>>
>>> diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
>>> index 8a659d8d47..f29b4c3d9a 100644
>>> --- a/docs/misra/rules.rst
>>> +++ b/docs/misra/rules.rst
>>> @@ -189,6 +189,12 @@ existing codebase are work-in-progress.
>>>         - A switch-expression shall not have essentially Boolean type
>>>         -
>>>    +   * - `Rule 17.1
>>> <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_17_01.c>`_
>>> +     - Required
>>> +     - The features of <stdarg.h> shall not be used
>>> +     - It is understood that in some limited circumstances <stdarg.h> is
>>> +       appropriate to use, such as the implementation of printk.
>>
>> The last bullet point is unclear to me. You don't define what "appropriate"
>> means here. So who is going to decide? Also, how is this going to be deviated?
>>
>> Possibly the solution here is to remove the last bullet point and have a
>> paragraph in deviations.rst explaining why we are using va_args.
> 
> Actually, I agree with you. I added the last bullet to address Jan's
> concern:
> https://marc.info/?l=xen-devel&m=170191695511513
> https://marc.info/?l=xen-devel&m=170193528120968
> 
> This was my original reply:
> 
> "We agreed that in certain situations stdarg.h is OK to use and in those
> cases we would add a deviation. Would you like me to add something to
> that effect here? I could do that but it would sound a bit vague.  Also
> if we want to specify a project-wide deviation it would be better
> documented in docs/misra/deviations.rst. I would leave Rule 17.1 without
> a note."
> 
> My preference is still to remove the last bullet (because too generic)
> and add any specific information to deviations.rst as usual.
> 
> Julien, would you be OK with this patch if I remove the last bullet and
> leave it blank?

I would be fine with that:

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall
Re: [PATCH v2] docs/misra/rules.rst: add Rule 17.1
Posted by Stefano Stabellini 4 months, 2 weeks ago 
On Tue, 19 Dec 2023, Julien Grall wrote:
> Hi Stefano,
> 
> On 15/12/2023 21:02, Stefano Stabellini wrote:
> > On Fri, 15 Dec 2023, Julien Grall wrote:
> > > Hi Stefano,
> > > 
> > > On 09/12/2023 01:39, Stefano Stabellini wrote:
> > > > 
> > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> > > > ---
> > > > Changes in v2:
> > > > - separated 17.1 in its own patch
> > > > - add a comment
> > > > 
> > > > diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
> > > > index 8a659d8d47..f29b4c3d9a 100644
> > > > --- a/docs/misra/rules.rst
> > > > +++ b/docs/misra/rules.rst
> > > > @@ -189,6 +189,12 @@ existing codebase are work-in-progress.
> > > >         - A switch-expression shall not have essentially Boolean type
> > > >         -
> > > >    +   * - `Rule 17.1
> > > > <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_17_01.c>`_
> > > > +     - Required
> > > > +     - The features of <stdarg.h> shall not be used
> > > > +     - It is understood that in some limited circumstances <stdarg.h>
> > > > is
> > > > +       appropriate to use, such as the implementation of printk.
> > > 
> > > The last bullet point is unclear to me. You don't define what
> > > "appropriate"
> > > means here. So who is going to decide? Also, how is this going to be
> > > deviated?
> > > 
> > > Possibly the solution here is to remove the last bullet point and have a
> > > paragraph in deviations.rst explaining why we are using va_args.
> > 
> > Actually, I agree with you. I added the last bullet to address Jan's
> > concern:
> > https://marc.info/?l=xen-devel&m=170191695511513
> > https://marc.info/?l=xen-devel&m=170193528120968
> > 
> > This was my original reply:
> > 
> > "We agreed that in certain situations stdarg.h is OK to use and in those
> > cases we would add a deviation. Would you like me to add something to
> > that effect here? I could do that but it would sound a bit vague.  Also
> > if we want to specify a project-wide deviation it would be better
> > documented in docs/misra/deviations.rst. I would leave Rule 17.1 without
> > a note."
> > 
> > My preference is still to remove the last bullet (because too generic)
> > and add any specific information to deviations.rst as usual.
> > 
> > Julien, would you be OK with this patch if I remove the last bullet and
> > leave it blank?
> 
> I would be fine with that:
> 
> Acked-by: Julien Grall <jgrall@amazon.com>

I plan to commit that one of the next few days unless someone speaks up

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.