Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

2024-04-24 Thread Jan Beulich
On 24.04.2024 09:37, Federico Serafini wrote:
> On 23/04/24 18:06, Jan Beulich wrote:
>> On 23.04.2024 17:52, Federico Serafini wrote:
>>> On 23/04/24 12:26, Jan Beulich wrote:
 On 23.04.2024 12:02, Federico Serafini wrote:
> +
> +   * - R16.4
> + - A switch statement with a single switch clause and no default 
> label may
> +   be used in place of an equivalent if statement if it is 
> considered to
> +   improve readability."
>>
>> No, I don't think there should be examples in those documents. But those
>> documents should also not (blindly) rely on terminology in the Misra
>> spec, as not everyone has access to that (licensed copies had to be
>> obtained for quite a few of us).
> 
> In deviations.rst there is an identical deviation for Rule 16.6
> ("Every switch statement shall have at least two switch-clauses").
> I think we should remain consistent.

Sure, I'm all for consistency. Yet given the term "switch clause" doesn't
appear in the C standard (afaics), it wants defining somewhere.

Jan



Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

2024-04-24 Thread Federico Serafini

On 23/04/24 18:06, Jan Beulich wrote:

On 23.04.2024 17:52, Federico Serafini wrote:

On 23/04/24 12:26, Jan Beulich wrote:

On 23.04.2024 12:02, Federico Serafini wrote:

+
+   * - R16.4
+ - A switch statement with a single switch clause and no default label may
+   be used in place of an equivalent if statement if it is considered to
+   improve readability."


No, I don't think there should be examples in those documents. But those
documents should also not (blindly) rely on terminology in the Misra
spec, as not everyone has access to that (licensed copies had to be
obtained for quite a few of us).


In deviations.rst there is an identical deviation for Rule 16.6
("Every switch statement shall have at least two switch-clauses").
I think we should remain consistent.

--
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)



Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

2024-04-23 Thread Jan Beulich
On 23.04.2024 17:52, Federico Serafini wrote:
> On 23/04/24 12:26, Jan Beulich wrote:
>> On 23.04.2024 12:02, Federico Serafini wrote:
>>> --- a/docs/misra/deviations.rst
>>> +++ b/docs/misra/deviations.rst
>>> @@ -302,6 +302,19 @@ Deviations related to MISRA C:2012 Rules:
>>>  leave such files as is.
>>>- Tagged as `deliberate` for ECLAIR.
>>>   
>>> +   * - R16.4
>>> + - Switch statements having a controlling expression of enum type
>>> +   deliberately do not have a default case: gcc -Wall enables -Wswitch
>>> +   which warns (and breaks the build as we use -Werror) if one of the 
>>> enum
>>> +   labels is missing from the switch.
>>> + - Tagged as `deliberate` for ECLAIR.
>>> +
>>> +   * - R16.4
>>> + - A switch statement with a single switch clause and no default label 
>>> may
>>> +   be used in place of an equivalent if statement if it is considered 
>>> to
>>> +   improve readability."
> 
> (I placed Rule 16.4 before Rule 16.3.
> I will propose a new version with the correct ordering.)
> 
>>
>> First a terminology related comment here: I'm afraid "switch clause" can be
>> interpreted multiple ways, when I think we want to leave no room for
>> interpretation here. It's not even clear to me whether
>>
>>  switch ( x )
>>  {
>>  case 1: case 2: case 3: case 4:
>>  ...
>>  break;
>>  }
>>
>> would be covered by the deviation, or whether the multiple case labels
>> wouldn't already be too much.
> 
> The MISRA C document, within Rule 16.1 ("A switch statement shall be
> well-formed") defines the syntax rules that can be used to define a
> "well formed" switch statement.
> When I say "switch clause", I refer to the same entity the MISRA
> document refers to in the definition of such syntax rules.
> In the example above, we have a single switch clause with multiple
> labels and no default label: this is a violation of Rule 16.4
> ("Every `switch' statement shall have a `default' label") which will
> be covered by the deviation.
> Do you think inserting the example in rules.rst or deviations.rst could
> be useful?

No, I don't think there should be examples in those documents. But those
documents should also not (blindly) rely on terminology in the Misra
spec, as not everyone has access to that (licensed copies had to be
obtained for quite a few of us).

Jan



Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

2024-04-23 Thread Federico Serafini

On 23/04/24 12:26, Jan Beulich wrote:

On 23.04.2024 12:02, Federico Serafini wrote:

--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -302,6 +302,19 @@ Deviations related to MISRA C:2012 Rules:
 leave such files as is.
   - Tagged as `deliberate` for ECLAIR.
  
+   * - R16.4

+ - Switch statements having a controlling expression of enum type
+   deliberately do not have a default case: gcc -Wall enables -Wswitch
+   which warns (and breaks the build as we use -Werror) if one of the enum
+   labels is missing from the switch.
+ - Tagged as `deliberate` for ECLAIR.
+
+   * - R16.4
+ - A switch statement with a single switch clause and no default label may
+   be used in place of an equivalent if statement if it is considered to
+   improve readability."


(I placed Rule 16.4 before Rule 16.3.
I will propose a new version with the correct ordering.)



First a terminology related comment here: I'm afraid "switch clause" can be
interpreted multiple ways, when I think we want to leave no room for
interpretation here. It's not even clear to me whether

 switch ( x )
 {
 case 1: case 2: case 3: case 4:
 ...
 break;
 }

would be covered by the deviation, or whether the multiple case labels
wouldn't already be too much.


The MISRA C document, within Rule 16.1 ("A switch statement shall be
well-formed") defines the syntax rules that can be used to define a
"well formed" switch statement.
When I say "switch clause", I refer to the same entity the MISRA
document refers to in the definition of such syntax rules.
In the example above, we have a single switch clause with multiple
labels and no default label: this is a violation of Rule 16.4
("Every `switch' statement shall have a `default' label") which will
be covered by the deviation.
Do you think inserting the example in rules.rst or deviations.rst could
be useful?



And then it is not clear to me why

 switch ( x )
 {
 case 1:
 ...
 break;
 default:
 ...
 break;
 }

shouldn't also be covered, as potentially a readability improvement /
future change simplification over

 if ( x == 1 )
 {
 ...
 }
 else
 {
 ...
 }


Here there are two switch clauses,
each of them terminated by a break statement,
and the default label is present:
the switch is well formed, no violations of series 16 will
be reported.

--
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)



Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

2024-04-23 Thread Jan Beulich
On 23.04.2024 12:02, Federico Serafini wrote:
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -302,6 +302,19 @@ Deviations related to MISRA C:2012 Rules:
> leave such files as is.
>   - Tagged as `deliberate` for ECLAIR.
>  
> +   * - R16.4
> + - Switch statements having a controlling expression of enum type
> +   deliberately do not have a default case: gcc -Wall enables -Wswitch
> +   which warns (and breaks the build as we use -Werror) if one of the 
> enum
> +   labels is missing from the switch.
> + - Tagged as `deliberate` for ECLAIR.
> +
> +   * - R16.4
> + - A switch statement with a single switch clause and no default label 
> may
> +   be used in place of an equivalent if statement if it is considered to
> +   improve readability."

First a terminology related comment here: I'm afraid "switch clause" can be
interpreted multiple ways, when I think we want to leave no room for
interpretation here. It's not even clear to me whether

switch ( x )
{
case 1: case 2: case 3: case 4:
...
break;
}

would be covered by the deviation, or whether the multiple case labels
wouldn't already be too much.

And then it is not clear to me why

switch ( x )
{
case 1:
...
break;
default:
...
break;
}

shouldn't also be covered, as potentially a readability improvement /
future change simplification over

if ( x == 1 )
{
...
}
else
{
...
}

Jan