Currently the script convert_misra_doc.py is using a loop through
range(1,22) to enumerate rules that needs to be skipped, however
range function does not include the stop counter in the enumeration
ending up into list rules until 21.21 instead of including rule 22.

Fix the issue using a dictionary that list the rules in misra c2012.

Fixes: 57caa5375321 ("xen: Add MISRA support to cppcheck make rule")
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
 xen/tools/convert_misra_doc.py | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/xen/tools/convert_misra_doc.py b/xen/tools/convert_misra_doc.py
index caa4487f645f..13074d8a2e91 100755
--- a/xen/tools/convert_misra_doc.py
+++ b/xen/tools/convert_misra_doc.py
@@ -14,6 +14,34 @@ Usage:
 
 import sys, getopt, re
 
+# MISRA rule are identified by two numbers, e.g. Rule 1.2, the main rule number
+# and a sub-number. This dictionary contains the number of the MISRA rule as key
+# and the maximum sub-number for that rule as value.
+misra_c2012_rules = {
+    1:4,
+    2:7,
+    3:2,
+    4:2,
+    5:9,
+    6:2,
+    7:4,
+    8:14,
+    9:5,
+    10:8,
+    11:9,
+    12:5,
+    13:6,
+    14:4,
+    15:7,
+    16:7,
+    17:8,
+    18:8,
+    19:2,
+    20:14,
+    21:21,
+    22:10
+}
+
 def main(argv):
     infile = ''
     outfile = ''
@@ -142,8 +170,8 @@ def main(argv):
     skip_list = []
 
     # Search for missing rules and add a dummy text with the rule number
-    for i in list(range(1,22)):
-        for j in list(range(1,22)):
+    for i in misra_c2012_rules:
+        for j in list(range(1,misra_c2012_rules[i]+1)):
             if str(i) + '.' + str(j) not in rule_list:
                 outstr.write('Rule ' + str(i) + '.' + str(j) + '\n')
                 outstr.write('No description for rule ' + str(i) + '.' + str(j)
-- 
2.17.1

On Mon, 28 Nov 2022, Luca Fancellu wrote:
> Currently the script convert_misra_doc.py is using a loop through
> range(1,22) to enumerate rules that needs to be skipped, however
> range function does not include the stop counter in the enumeration
> ending up into list rules until 21.21 instead of including rule 22.
> 
> Fix the issue using a dictionary that list the rules in misra c2012.

I think I understand the problem you are trying to solve with this
patch. But I am confused about the proposed solution.

The original code is trying to list all the possible MISRA C rules that
are not in docs/misra/rules.rst. Instead of list(range(1,22)) now we
have a dictionary: misra_c2012_rules. But misra_c2012_rules doesn't have
all the possible MISRA C rules missing from docs/misra/rules.rst.

As an example Rule 13.1 is missing from docs/misra/rules.rst but it is
also missing from misra_c2012_rules.

Can you please help me understand why misra_c2012_rules has only a small
subset of MISRA C rules to be skipped?


> Fixes: 57caa5375321 ("xen: Add MISRA support to cppcheck make rule")
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> ---
>  xen/tools/convert_misra_doc.py | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/tools/convert_misra_doc.py b/xen/tools/convert_misra_doc.py
> index caa4487f645f..13074d8a2e91 100755
> --- a/xen/tools/convert_misra_doc.py
> +++ b/xen/tools/convert_misra_doc.py
> @@ -14,6 +14,34 @@ Usage:
>  
>  import sys, getopt, re
>  
> +# MISRA rule are identified by two numbers, e.g. Rule 1.2, the main rule number
> +# and a sub-number. This dictionary contains the number of the MISRA rule as key
> +# and the maximum sub-number for that rule as value.
> +misra_c2012_rules = {
> +    1:4,
> +    2:7,
> +    3:2,
> +    4:2,
> +    5:9,
> +    6:2,
> +    7:4,
> +    8:14,
> +    9:5,
> +    10:8,
> +    11:9,
> +    12:5,
> +    13:6,
> +    14:4,
> +    15:7,
> +    16:7,
> +    17:8,
> +    18:8,
> +    19:2,
> +    20:14,
> +    21:21,
> +    22:10
> +}
> +
>  def main(argv):
>      infile = ''
>      outfile = ''
> @@ -142,8 +170,8 @@ def main(argv):
>      skip_list = []
>  
>      # Search for missing rules and add a dummy text with the rule number
> -    for i in list(range(1,22)):
> -        for j in list(range(1,22)):
> +    for i in misra_c2012_rules:
> +        for j in list(range(1,misra_c2012_rules[i]+1)):
>              if str(i) + '.' + str(j) not in rule_list:
>                  outstr.write('Rule ' + str(i) + '.' + str(j) + '\n')
>                  outstr.write('No description for rule ' + str(i) + '.' + str(j)
> -- 
> 2.17.1
>

> On 29 Nov 2022, at 23:51, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Mon, 28 Nov 2022, Luca Fancellu wrote:
>> Currently the script convert_misra_doc.py is using a loop through
>> range(1,22) to enumerate rules that needs to be skipped, however
>> range function does not include the stop counter in the enumeration
>> ending up into list rules until 21.21 instead of including rule 22.
>> 
>> Fix the issue using a dictionary that list the rules in misra c2012.
> 
> I think I understand the problem you are trying to solve with this
> patch. But I am confused about the proposed solution.
> 
> The original code is trying to list all the possible MISRA C rules that
> are not in docs/misra/rules.rst. Instead of list(range(1,22)) now we
> have a dictionary: misra_c2012_rules. But misra_c2012_rules doesn't have
> all the possible MISRA C rules missing from docs/misra/rules.rst.
> 
> As an example Rule 13.1 is missing from docs/misra/rules.rst but it is
> also missing from misra_c2012_rules.
> 
> Can you please help me understand why misra_c2012_rules has only a small
> subset of MISRA C rules to be skipped?

Hi Stefano,

MISRA rules are in this format X.Y, misra_c2012_rules is a dictionary where the key is 
X and the value is the maximum number that Y can have.

For example rule 13.Y goes from 13.1 to 13.6 (in the dictionary misra_c2012_rules[13] == 6),
so the code can now check which among (13.1 .. 13.6) is not in the rule_list and add it to the
list of skipped rules.

Here an example:
{
    "script": "misra.py",
    "args": [
      "--rule-texts=/path/to/cppcheck-misra.txt",
      "--suppress-rules=1.1,1.2,1.4,2.2,2.3,2.4,2.5,2.6,2.7,3.1,4.1,4.2,5.5,5.6,5.7,5.8,5.9,6.1,7.1,7.2,7.3,7.4,8.2,8.3,8.7,8.9,8.11,8.13,8.14,9.3,9.4,9.5,10.1,10.2,10.3,10.4,10.5,10.6,10.7,10.8,11.1,11.2,11.3,11.4,11.5,11.6,11.7,11.8,11.9,12.1,12.2,12.3,12.4,12.5,13.1,13.2,13.3,13.4,13.5,14.2,14.3,14.4,15.1,15.2,15.3,15.4,15.5,15.6,15.7,16.1,16.2,16.3,16.4,16.5,16.6,17.1,17.2,17.5,17.6,17.7,17.8,18.1,18.2,18.3,18.4,18.5,18.6,18.7,18.8,19.1,19.2,20.1,20.2,20.3,20.4,20.5,20.6,20.8,20.9,20.10,20.11,20.12,21.1,21.2,21.3,21.4,21.5,21.6,21.7,21.8,21.9,21.10,21.11,21.12,21.13,21.14,21.15,21.16,21.17,21.18,21.19,21.20,21.21,22.1,22.2,22.3,22.4,22.5,22.6,22.7,22.8,22.9,22.10"
    ]
}

So this patch is solving two issues, the first one was that rule 22.Y was never included in the suppressed
list because range(1,22) produces a range in [1..21], the second issue is that the code was producing
Invalid MISRA C 2012 rules, for example 1.21 and so on.


> 
> 
>> Fixes: 57caa5375321 ("xen: Add MISRA support to cppcheck make rule")
>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>> ---
>> xen/tools/convert_misra_doc.py | 32 ++++++++++++++++++++++++++++++--
>> 1 file changed, 30 insertions(+), 2 deletions(-)
>> 
>> diff --git a/xen/tools/convert_misra_doc.py b/xen/tools/convert_misra_doc.py
>> index caa4487f645f..13074d8a2e91 100755
>> --- a/xen/tools/convert_misra_doc.py
>> +++ b/xen/tools/convert_misra_doc.py
>> @@ -14,6 +14,34 @@ Usage:
>> 
>> import sys, getopt, re
>> 
>> +# MISRA rule are identified by two numbers, e.g. Rule 1.2, the main rule number
>> +# and a sub-number. This dictionary contains the number of the MISRA rule as key
>> +# and the maximum sub-number for that rule as value.
>> +misra_c2012_rules = {
>> +    1:4,
>> +    2:7,
>> +    3:2,
>> +    4:2,
>> +    5:9,
>> +    6:2,
>> +    7:4,
>> +    8:14,
>> +    9:5,
>> +    10:8,
>> +    11:9,
>> +    12:5,
>> +    13:6,
>> +    14:4,
>> +    15:7,
>> +    16:7,
>> +    17:8,
>> +    18:8,
>> +    19:2,
>> +    20:14,
>> +    21:21,
>> +    22:10
>> +}
>> +
>> def main(argv):
>>     infile = ''
>>     outfile = ''
>> @@ -142,8 +170,8 @@ def main(argv):
>>     skip_list = []
>> 
>>     # Search for missing rules and add a dummy text with the rule number
>> -    for i in list(range(1,22)):
>> -        for j in list(range(1,22)):
>> +    for i in misra_c2012_rules:
>> +        for j in list(range(1,misra_c2012_rules[i]+1)):
>>             if str(i) + '.' + str(j) not in rule_list:
>>                 outstr.write('Rule ' + str(i) + '.' + str(j) + '\n')
>>                 outstr.write('No description for rule ' + str(i) + '.' + str(j)
>> -- 
>> 2.17.1
>> 

On Wed, 30 Nov 2022, Luca Fancellu wrote:
> > On 29 Nov 2022, at 23:51, Stefano Stabellini <sstabellini@kernel.org> wrote:
> > 
> > On Mon, 28 Nov 2022, Luca Fancellu wrote:
> >> Currently the script convert_misra_doc.py is using a loop through
> >> range(1,22) to enumerate rules that needs to be skipped, however
> >> range function does not include the stop counter in the enumeration
> >> ending up into list rules until 21.21 instead of including rule 22.
> >> 
> >> Fix the issue using a dictionary that list the rules in misra c2012.
> > 
> > I think I understand the problem you are trying to solve with this
> > patch. But I am confused about the proposed solution.
> > 
> > The original code is trying to list all the possible MISRA C rules that
> > are not in docs/misra/rules.rst. Instead of list(range(1,22)) now we
> > have a dictionary: misra_c2012_rules. But misra_c2012_rules doesn't have
> > all the possible MISRA C rules missing from docs/misra/rules.rst.
> > 
> > As an example Rule 13.1 is missing from docs/misra/rules.rst but it is
> > also missing from misra_c2012_rules.
> > 
> > Can you please help me understand why misra_c2012_rules has only a small
> > subset of MISRA C rules to be skipped?
> 
> Hi Stefano,
> 
> MISRA rules are in this format X.Y, misra_c2012_rules is a dictionary where the key is 
> X and the value is the maximum number that Y can have.
> 
> For example rule 13.Y goes from 13.1 to 13.6 (in the dictionary misra_c2012_rules[13] == 6),
> so the code can now check which among (13.1 .. 13.6) is not in the rule_list and add it to the
> list of skipped rules.
> 
> Here an example:
> {
>     "script": "misra.py",
>     "args": [
>       "--rule-texts=/path/to/cppcheck-misra.txt",
>       "--suppress-rules=1.1,1.2,1.4,2.2,2.3,2.4,2.5,2.6,2.7,3.1,4.1,4.2,5.5,5.6,5.7,5.8,5.9,6.1,7.1,7.2,7.3,7.4,8.2,8.3,8.7,8.9,8.11,8.13,8.14,9.3,9.4,9.5,10.1,10.2,10.3,10.4,10.5,10.6,10.7,10.8,11.1,11.2,11.3,11.4,11.5,11.6,11.7,11.8,11.9,12.1,12.2,12.3,12.4,12.5,13.1,13.2,13.3,13.4,13.5,14.2,14.3,14.4,15.1,15.2,15.3,15.4,15.5,15.6,15.7,16.1,16.2,16.3,16.4,16.5,16.6,17.1,17.2,17.5,17.6,17.7,17.8,18.1,18.2,18.3,18.4,18.5,18.6,18.7,18.8,19.1,19.2,20.1,20.2,20.3,20.4,20.5,20.6,20.8,20.9,20.10,20.11,20.12,21.1,21.2,21.3,21.4,21.5,21.6,21.7,21.8,21.9,21.10,21.11,21.12,21.13,21.14,21.15,21.16,21.17,21.18,21.19,21.20,21.21,22.1,22.2,22.3,22.4,22.5,22.6,22.7,22.8,22.9,22.10"
>     ]
> }
> 
> So this patch is solving two issues, the first one was that rule 22.Y was never included in the suppressed
> list because range(1,22) produces a range in [1..21], the second issue is that the code was producing
> Invalid MISRA C 2012 rules, for example 1.21 and so on.

I see, that makes sense. Please improve the commit message with this
information and add

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

>> 
>> Hi Stefano,
>> 
>> MISRA rules are in this format X.Y, misra_c2012_rules is a dictionary where the key is 
>> X and the value is the maximum number that Y can have.
>> 
>> For example rule 13.Y goes from 13.1 to 13.6 (in the dictionary misra_c2012_rules[13] == 6),
>> so the code can now check which among (13.1 .. 13.6) is not in the rule_list and add it to the
>> list of skipped rules.
>> 
>> Here an example:
>> {
>>    "script": "misra.py",
>>    "args": [
>>      "--rule-texts=/path/to/cppcheck-misra.txt",
>>      "--suppress-rules=1.1,1.2,1.4,2.2,2.3,2.4,2.5,2.6,2.7,3.1,4.1,4.2,5.5,5.6,5.7,5.8,5.9,6.1,7.1,7.2,7.3,7.4,8.2,8.3,8.7,8.9,8.11,8.13,8.14,9.3,9.4,9.5,10.1,10.2,10.3,10.4,10.5,10.6,10.7,10.8,11.1,11.2,11.3,11.4,11.5,11.6,11.7,11.8,11.9,12.1,12.2,12.3,12.4,12.5,13.1,13.2,13.3,13.4,13.5,14.2,14.3,14.4,15.1,15.2,15.3,15.4,15.5,15.6,15.7,16.1,16.2,16.3,16.4,16.5,16.6,17.1,17.2,17.5,17.6,17.7,17.8,18.1,18.2,18.3,18.4,18.5,18.6,18.7,18.8,19.1,19.2,20.1,20.2,20.3,20.4,20.5,20.6,20.8,20.9,20.10,20.11,20.12,21.1,21.2,21.3,21.4,21.5,21.6,21.7,21.8,21.9,21.10,21.11,21.12,21.13,21.14,21.15,21.16,21.17,21.18,21.19,21.20,21.21,22.1,22.2,22.3,22.4,22.5,22.6,22.7,22.8,22.9,22.10"
>>    ]
>> }
>> 
>> So this patch is solving two issues, the first one was that rule 22.Y was never included in the suppressed
>> list because range(1,22) produces a range in [1..21], the second issue is that the code was producing
>> Invalid MISRA C 2012 rules, for example 1.21 and so on.
> 
> I see, that makes sense. Please improve the commit message with this
> information and add
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Thank you,

If you agree, I will change the commit message to be this one:

MISRA rules are in the format Rule X.Y, currently the script
convert_misra_doc.py is using two nested loop through range(1,22) to
enumerate rules that needs to be skipped, using combination of X.Y in
that range, however there are two issues in the code:
 - rule 22 is never included because the range(1,22) produces a range 
   in [1..21]
 - the second issue is that the code is producing invalid MISRA C 2012
   rules, for example 1.21 and so on

Fix the issue using a dictionary that list the rules in misra c2012.

On Thu, 1 Dec 2022, Luca Fancellu wrote:
> >> Hi Stefano,
> >> 
> >> MISRA rules are in this format X.Y, misra_c2012_rules is a dictionary where the key is 
> >> X and the value is the maximum number that Y can have.
> >> 
> >> For example rule 13.Y goes from 13.1 to 13.6 (in the dictionary misra_c2012_rules[13] == 6),
> >> so the code can now check which among (13.1 .. 13.6) is not in the rule_list and add it to the
> >> list of skipped rules.
> >> 
> >> Here an example:
> >> {
> >>    "script": "misra.py",
> >>    "args": [
> >>      "--rule-texts=/path/to/cppcheck-misra.txt",
> >>      "--suppress-rules=1.1,1.2,1.4,2.2,2.3,2.4,2.5,2.6,2.7,3.1,4.1,4.2,5.5,5.6,5.7,5.8,5.9,6.1,7.1,7.2,7.3,7.4,8.2,8.3,8.7,8.9,8.11,8.13,8.14,9.3,9.4,9.5,10.1,10.2,10.3,10.4,10.5,10.6,10.7,10.8,11.1,11.2,11.3,11.4,11.5,11.6,11.7,11.8,11.9,12.1,12.2,12.3,12.4,12.5,13.1,13.2,13.3,13.4,13.5,14.2,14.3,14.4,15.1,15.2,15.3,15.4,15.5,15.6,15.7,16.1,16.2,16.3,16.4,16.5,16.6,17.1,17.2,17.5,17.6,17.7,17.8,18.1,18.2,18.3,18.4,18.5,18.6,18.7,18.8,19.1,19.2,20.1,20.2,20.3,20.4,20.5,20.6,20.8,20.9,20.10,20.11,20.12,21.1,21.2,21.3,21.4,21.5,21.6,21.7,21.8,21.9,21.10,21.11,21.12,21.13,21.14,21.15,21.16,21.17,21.18,21.19,21.20,21.21,22.1,22.2,22.3,22.4,22.5,22.6,22.7,22.8,22.9,22.10"
> >>    ]
> >> }
> >> 
> >> So this patch is solving two issues, the first one was that rule 22.Y was never included in the suppressed
> >> list because range(1,22) produces a range in [1..21], the second issue is that the code was producing
> >> Invalid MISRA C 2012 rules, for example 1.21 and so on.
> > 
> > I see, that makes sense. Please improve the commit message with this
> > information and add
> > 
> > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> Thank you,
> 
> If you agree, I will change the commit message to be this one:
> 
> MISRA rules are in the format Rule X.Y, currently the script
> convert_misra_doc.py is using two nested loop through range(1,22) to
> enumerate rules that needs to be skipped, using combination of X.Y in
> that range, however there are two issues in the code:
>  - rule 22 is never included because the range(1,22) produces a range 
>    in [1..21]
>  - the second issue is that the code is producing invalid MISRA C 2012
>    rules, for example 1.21 and so on
> 
> Fix the issue using a dictionary that list the rules in misra c2012.

Sounds good