| Message ID | alpine.DEB.2.22.394.2312081738100.1703076@ubuntu-linux-20-04-desktop (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] docs/misra/rules.rst: add Rule 17.1 | expand |
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,
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?
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,
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
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
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> --- Changes in v2: - separated 17.1 in its own patch - add a comment