| Message ID | 20240725075830.63585-1-sunjunchao2870@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | scripts: reduce false positives in the macro_checker script. | expand |
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. Honza
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? > > Honza > -- > Jan Kara <jack@suse.com> > SUSE Labs, CR Thanks,
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(-)