diff mbox series

scripts: reduce false positives in the macro_checker script.

Message ID 20240725075830.63585-1-sunjunchao2870@gmail.com
State New
Headers show
Series scripts: reduce false positives in the macro_checker script. | expand

Commit Message

Julian Sun July 25, 2024, 7:58 a.m. UTC
Reduce false positives in the macro_checker
in the following scenarios:
  1. Conditional compilation
  2. Macro definitions with only a single character
  3. Macro definitions as (0) and (1)

Before this patch:
	sjc@sjc:linux$ ./scripts/macro_checker.py  fs | wc -l
	99

After this patch:
	sjc@sjc:linux$ ./scripts/macro_checker.py  fs | wc -l
	11

Most of the current warnings are valid now.

Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
---
 scripts/macro_checker.py | 37 ++++++++++++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 5 deletions(-)

Comments

Jan Kara July 25, 2024, 9:48 a.m. UTC | #1
On Thu 25-07-24 05:15:34, Julian Sun wrote:
> Jan Kara <jack@suse.cz> 于2024年7月25日周四 04:52写道:
> >
> > On Thu 25-07-24 03:58:30, Julian Sun wrote:
> > > Reduce false positives in the macro_checker
> > > in the following scenarios:
> > >   1. Conditional compilation
> > >   2. Macro definitions with only a single character
> > >   3. Macro definitions as (0) and (1)
> > >
> > > Before this patch:
> > >       sjc@sjc:linux$ ./scripts/macro_checker.py  fs | wc -l
> > >       99
> > >
> > > After this patch:
> > >       sjc@sjc:linux$ ./scripts/macro_checker.py  fs | wc -l
> > >       11
> > >
> > > Most of the current warnings are valid now.
> > >
> > > Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
> > ...
> > >  def file_check_macro(file_path, report):
> > > +    # number of conditional compiling
> > > +    cond_compile = 0
> > >      # only check .c and .h file
> > >      if not file_path.endswith(".c") and not file_path.endswith(".h"):
> > >          return
> > > @@ -57,7 +72,14 @@ def file_check_macro(file_path, report):
> > >          while True:
> > >              line = f.readline()
> > >              if not line:
> > > -                return
> > > +                break
> > > +            line = line.strip()
> > > +            if line.startswith(cond_compile_mark):
> > > +                cond_compile += 1
> > > +                continue
> > > +            if line.startswith(cond_compile_end):
> > > +                cond_compile -= 1
> > > +                continue
> > >
> > >              macro = re.match(macro_pattern, line)
> > >              if macro:
> > > @@ -67,6 +89,11 @@ def file_check_macro(file_path, report):
> > >                      macro = macro.strip()
> > >                      macro += f.readline()
> > >                      macro = macro_strip(macro)
> > > +                if file_path.endswith(".c")  and cond_compile != 0:
> > > +                    continue
> > > +                # 1 is for #ifdef xxx at the beginning of the header file
> > > +                if file_path.endswith(".h") and cond_compile != 1:
> > > +                    continue
> > >                  check_macro(macro, report)
> > >
> > >  def get_correct_macros(path):
> >
> >
> > > So I don't think this is right. As far as I understand this skips any macros
> > > that are conditionally defined? Why? There is a lot of them and checking
> > > them is beneficial... The patterns you have added should be dealing with
> > > most of the conditional defines anyway.
> Yes, this skips all checks for conditional macro. This is because I
> observed that almost all false positives come from conditional
> compilation. Testing showed that skipping them does not cause the
> genuine warnings to disappear.
> Also as you said, it may still lead to skipping checks for genuinely
> problematic macro definitions. Perhaps we could provide an option that
> allows users to control whether or not to check macros under
> conditional compilation?

Yes, that could be useful.

								Honza
diff mbox series

Patch

diff --git a/scripts/macro_checker.py b/scripts/macro_checker.py
index cd10c9c10d31..8195339ea5b5 100755
--- a/scripts/macro_checker.py
+++ b/scripts/macro_checker.py
@@ -9,9 +9,12 @@  import os
 import re
 
 macro_pattern = r"#define\s+(\w+)\(([^)]*)\)"
-# below two vars were used to reduce false positives
-do_while0_pattern = r"\s*do\s*\{\s*\}\s*while\s*\(\s*0\s*\)"
+# below vars were used to reduce false positives
+fp_patterns = [r"\s*do\s*\{\s*\}\s*while\s*\(\s*0\s*\)",
+               r"\(?0\)?", r"\(?1\)?"]
 correct_macros = []
+cond_compile_mark = "#if"
+cond_compile_end = "#endif"
 
 def check_macro(macro_line, report):
     match = re.match(macro_pattern, macro_line)
@@ -21,15 +24,25 @@  def check_macro(macro_line, report):
         content = match.group(2)
         arguments = [item.strip() for item in content.split(',') if item.strip()]
 
-        if (re.match(do_while0_pattern, macro_def)):
+        macro_def = macro_def.strip()
+        if not macro_def:
             return
+        # used to reduce false positives, like #define endfor_nexthops(rt) }
+        if len(macro_def) == 1:
+            return
+
+        for fp_pattern in fp_patterns:
+            if (re.match(fp_pattern, macro_def)):
+                return
 
         for arg in arguments:
             # used to reduce false positives
             if "..." in arg:
-                continue
+                return
+        for arg in arguments:
             if not arg in macro_def and report == False:
                 return
+            # if there is a correct macro with the same name, do not report it.
             if not arg in macro_def and identifier not in correct_macros:
                 print(f"Argument {arg} is not used in function-line macro {identifier}")
                 return
@@ -49,6 +62,8 @@  def macro_strip(macro):
     return macro
 
 def file_check_macro(file_path, report):
+    # number of conditional compiling
+    cond_compile = 0
     # only check .c and .h file
     if not file_path.endswith(".c") and not file_path.endswith(".h"):
         return
@@ -57,7 +72,14 @@  def file_check_macro(file_path, report):
         while True:
             line = f.readline()
             if not line:
-                return
+                break
+            line = line.strip()
+            if line.startswith(cond_compile_mark):
+                cond_compile += 1
+                continue
+            if line.startswith(cond_compile_end):
+                cond_compile -= 1
+                continue
 
             macro = re.match(macro_pattern, line)
             if macro:
@@ -67,6 +89,11 @@  def file_check_macro(file_path, report):
                     macro = macro.strip()
                     macro += f.readline()
                     macro = macro_strip(macro)
+                if file_path.endswith(".c")  and cond_compile != 0:
+                    continue
+                # 1 is for #ifdef xxx at the beginning of the header file
+                if file_path.endswith(".h") and cond_compile != 1:
+                    continue
                 check_macro(macro, report)
 
 def get_correct_macros(path):