diff mbox series

docs/misra: add rules 10.1 10.2 10.3 10.4

Message ID 20230823231451.2989262-1-sstabellini@kernel.org (mailing list archive)
State Superseded
Headers show
Series docs/misra: add rules 10.1 10.2 10.3 10.4 | expand

Commit Message

Stefano Stabellini Aug. 23, 2023, 11:14 p.m. UTC
From: Stefano Stabellini <stefano.stabellini@amd.com>

10.1 with several caveats, described in the notes.
10.3 and 10.4 as "aspirational" guidelines, as clarified in the notes.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
 docs/misra/rules.rst | 52 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

Comments

Jan Beulich Aug. 25, 2023, 1:40 p.m. UTC | #1
On 24.08.2023 01:14, Stefano Stabellini wrote:
> --- a/docs/misra/rules.rst
> +++ b/docs/misra/rules.rst
> @@ -318,6 +318,58 @@ maintainers if you want to suggest a change.
>       - An element of an object shall not be initialized more than once
>       -
>  
> +   * - `Rule 10.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_10_01.c>`_
> +     - Required
> +     - Operands shall not be of an inappropriate essential type
> +     - The following are allowed:
> +         - Value-preserving conversions of integer constants
> +         - Bitwise and, or, xor, one's complement, bitwise and assignment,
> +           bitwise or assignment, bitwise xor assignment (bitwise and, or, xor
> +           are safe on non-negative integers; also Xen assumes two's complement
> +           representation)
> +         - Left shift, right shift, left shift assignment, right shift
> +           assignment (see C-language-toolchain.rst for assumptions on
> +           compilers' extensions)

Is "assumptions" the right term here? We don't just assume these are there,
we actually checked their doc and behavior. Maybe simply "uses of" instead?

> +         - Implicit conversions to boolean for logical operators' arguments

What is "logical operators" here? Perhaps this wants to be "conditionals"
instead, to cover all of ?:, if(), while(), for() (did I forget any?), of
which only the first is an operator?

> +   * - `Rule 10.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_10_03.c>`_
> +     - Required
> +     - The value of an expression shall not be assigned to an object
> +       with a narrower essential type or of a dierent essential type

Nit: ff missing?

> +       category
> +     - Please beware that this rule has many violations in the Xen
> +       codebase today, and its adoption is aspirational. However, when
> +       submitting new patches please try to decrease the number of
> +       violations when possible.
> +
> +       gcc has a helpful warning that can help you spot and remove
> +       violations of this kind: conversion. For instance, you can use
> +       it as follows:
> +
> +       cd xen; CFLAGS="-Wconversion -Wno-error=sign-conversion -Wno-error=conversion" make

Maybe slightly shorter as

CFLAGS="-Wconversion -Wno-error=sign-conversion -Wno-error=conversion" make -C xen

?

Jan
Julien Grall Aug. 25, 2023, 1:44 p.m. UTC | #2
On 25/08/2023 14:40, Jan Beulich wrote:
> On 24.08.2023 01:14, Stefano Stabellini wrote:
>> --- a/docs/misra/rules.rst
>> +++ b/docs/misra/rules.rst
>> @@ -318,6 +318,58 @@ maintainers if you want to suggest a change.
>>        - An element of an object shall not be initialized more than once
>>        -
>>   
>> +   * - `Rule 10.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_10_01.c>`_
>> +     - Required
>> +     - Operands shall not be of an inappropriate essential type
>> +     - The following are allowed:
>> +         - Value-preserving conversions of integer constants
>> +         - Bitwise and, or, xor, one's complement, bitwise and assignment,
>> +           bitwise or assignment, bitwise xor assignment (bitwise and, or, xor
>> +           are safe on non-negative integers; also Xen assumes two's complement
>> +           representation)
>> +         - Left shift, right shift, left shift assignment, right shift
>> +           assignment (see C-language-toolchain.rst for assumptions on
>> +           compilers' extensions)
> 
> Is "assumptions" the right term here? We don't just assume these are there,
> we actually checked their doc and behavior. Maybe simply "uses of" instead?
> 
>> +         - Implicit conversions to boolean for logical operators' arguments
> 
> What is "logical operators" here? Perhaps this wants to be "conditionals"
> instead, to cover all of ?:, if(), while(), for() (did I forget any?), of
> which only the first is an operator?
> 
>> +   * - `Rule 10.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_10_03.c>`_
>> +     - Required
>> +     - The value of an expression shall not be assigned to an object
>> +       with a narrower essential type or of a dierent essential type
> 
> Nit: ff missing?
> 
>> +       category
>> +     - Please beware that this rule has many violations in the Xen
>> +       codebase today, and its adoption is aspirational. However, when
>> +       submitting new patches please try to decrease the number of
>> +       violations when possible.
>> +
>> +       gcc has a helpful warning that can help you spot and remove
>> +       violations of this kind: conversion. For instance, you can use
>> +       it as follows:
>> +
>> +       cd xen; CFLAGS="-Wconversion -Wno-error=sign-conversion -Wno-error=conversion" make
> 
> Maybe slightly shorter as
> 
> CFLAGS="-Wconversion -Wno-error=sign-conversion -Wno-error=conversion" make -C xen
> 
> ?

+1. It also means that the command can be called multiple time without 
having to type 'cd -' between.

Cheers,
Stefano Stabellini Aug. 25, 2023, 9:48 p.m. UTC | #3
On Fri, 25 Aug 2023, Jan Beulich wrote:
> On 24.08.2023 01:14, Stefano Stabellini wrote:
> > --- a/docs/misra/rules.rst
> > +++ b/docs/misra/rules.rst
> > @@ -318,6 +318,58 @@ maintainers if you want to suggest a change.
> >       - An element of an object shall not be initialized more than once
> >       -
> >  
> > +   * - `Rule 10.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_10_01.c>`_
> > +     - Required
> > +     - Operands shall not be of an inappropriate essential type
> > +     - The following are allowed:
> > +         - Value-preserving conversions of integer constants
> > +         - Bitwise and, or, xor, one's complement, bitwise and assignment,
> > +           bitwise or assignment, bitwise xor assignment (bitwise and, or, xor
> > +           are safe on non-negative integers; also Xen assumes two's complement
> > +           representation)
> > +         - Left shift, right shift, left shift assignment, right shift
> > +           assignment (see C-language-toolchain.rst for assumptions on
> > +           compilers' extensions)
> 
> Is "assumptions" the right term here? We don't just assume these are there,
> we actually checked their doc and behavior. Maybe simply "uses of" instead?

yes, I'll use "uses of"


> > +         - Implicit conversions to boolean for logical operators' arguments
> 
> What is "logical operators" here? Perhaps this wants to be "conditionals"
> instead, to cover all of ?:, if(), while(), for() (did I forget any?), of
> which only the first is an operator?

There are also ! || && which are the logical operators

I'll write it as follows:

Implicit conversions to boolean for conditionals (?: if
while for) and logical operators (! || &&)


> > +   * - `Rule 10.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_10_03.c>`_
> > +     - Required
> > +     - The value of an expression shall not be assigned to an object
> > +       with a narrower essential type or of a dierent essential type
> 
> Nit: ff missing?

yep, thanks


> > +       category
> > +     - Please beware that this rule has many violations in the Xen
> > +       codebase today, and its adoption is aspirational. However, when
> > +       submitting new patches please try to decrease the number of
> > +       violations when possible.
> > +
> > +       gcc has a helpful warning that can help you spot and remove
> > +       violations of this kind: conversion. For instance, you can use
> > +       it as follows:
> > +
> > +       cd xen; CFLAGS="-Wconversion -Wno-error=sign-conversion -Wno-error=conversion" make
> 
> Maybe slightly shorter as
> 
> CFLAGS="-Wconversion -Wno-error=sign-conversion -Wno-error=conversion" make -C xen
> 
> ?

I'll make the change here and also in the other instance of the same
diff mbox series

Patch

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 4f33ed4ba6..2f16451703 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -318,6 +318,58 @@  maintainers if you want to suggest a change.
      - An element of an object shall not be initialized more than once
      -
 
+   * - `Rule 10.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_10_01.c>`_
+     - Required
+     - Operands shall not be of an inappropriate essential type
+     - The following are allowed:
+         - Value-preserving conversions of integer constants
+         - Bitwise and, or, xor, one's complement, bitwise and assignment,
+           bitwise or assignment, bitwise xor assignment (bitwise and, or, xor
+           are safe on non-negative integers; also Xen assumes two's complement
+           representation)
+         - Left shift, right shift, left shift assignment, right shift
+           assignment (see C-language-toolchain.rst for assumptions on
+           compilers' extensions)
+         - Implicit conversions to boolean for logical operators' arguments
+
+   * - `Rule 10.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_10_02.c>`_
+     - Required
+     - Expressions of essentially character type shall not be used
+       inappropriately in addition and subtraction operations
+     -
+
+   * - `Rule 10.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_10_03.c>`_
+     - Required
+     - The value of an expression shall not be assigned to an object
+       with a narrower essential type or of a dierent essential type
+       category
+     - Please beware that this rule has many violations in the Xen
+       codebase today, and its adoption is aspirational. However, when
+       submitting new patches please try to decrease the number of
+       violations when possible.
+
+       gcc has a helpful warning that can help you spot and remove
+       violations of this kind: conversion. For instance, you can use
+       it as follows:
+
+       cd xen; CFLAGS="-Wconversion -Wno-error=sign-conversion -Wno-error=conversion" make
+
+   * - `Rule 10.4 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_10_04.c>`_
+     - Required
+     - Both operands of an operator in which the usual arithmetic
+       conversions are performed shall have the same essential type
+       category
+     - Please beware that this rule has many violations in the Xen
+       codebase today, and its adoption is aspirational. However, when
+       submitting new patches please try to decrease the number of
+       violations when possible.
+
+       gcc has a helpful warning that can help you spot and remove
+       violations of this kind: arith-conversion. For instance, you
+       can use it as follows:
+
+       cd xen; CFLAGS="-Warith-conversion -Wno-error=arith-conversion" make
+
    * - `Rule 12.5 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_12_05.c>`_
      - Mandatory
      - The sizeof operator shall not have an operand which is a function