| Message ID | 20240725075830.63585-1-sunjunchao2870@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | scripts: reduce false positives in the macro_checker script. | expand |
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 --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):
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(-)