From 75195855470cff941576b5dfff7daca4dea1e9f4 Mon Sep 17 00:00:00 2001 From: Xavier Simonart Date: Wed, 8 Jan 2025 12:34:17 -0500 Subject: [PATCH] checkpatch: Ensure ovn-nbctl/sbctl commands have a check. ovn-sbctl and ovn-nbctl commands should have a check in tests, except the commands "find", "show", "list", "get" and "dump-flows". Commands displaying an uuid, such as "create" should be prepended by check_uuid. This will ensure that those commands succeeds and tests behave as expected. Multiline commands (ending with '\') in tests are now checked as single lines to be able to detect potential "list", "find", ... commands. Signed-off-by: Xavier Simonart Signed-off-by: 0-day Robot --- tests/checkpatch.at | 65 +++++++++++++++++++++++++++++++++++++++++ utilities/checkpatch.py | 40 +++++++++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/tests/checkpatch.at b/tests/checkpatch.at index 6ac0e51f31..b874b1ea6e 100755 --- a/tests/checkpatch.at +++ b/tests/checkpatch.at @@ -644,3 +644,68 @@ try_checkpatch \ " AT_CLEANUP + +AT_SETUP([checkpatch - check and check_uuid]) +# Verify that missing check is covered. +try_checkpatch \ + "COMMON_PATCH_HEADER([tests/something.at]) + +ovn-nbctl lsp-add ls + " \ + "WARNING: ovn-nbctl return should be checked Consider adding check or check_uuid in front. + #8 FILE: tests/something.at:1: + ovn-nbctl lsp-add ls +" + +# Verify that existing check makes checkpatch happy. +try_checkpatch \ + "COMMON_PATCH_HEADER([tests/something.at]) + +check ovn-nbctl lsp-add ls + " + +# Verify that missing check_uuid is covered. +try_checkpatch \ + "COMMON_PATCH_HEADER([tests/something.at]) + +ovn-sbctl -- create FDB mac='"00\:00\:00\:00\:00\:03"' dp_key=1 port_key=1 + " \ + "WARNING: ovn-sbctl return should be checked Consider adding check or check_uuid in front. + #8 FILE: tests/something.at:1: + ovn-sbctl -- create FDB mac='00:00:00:00:00:03' dp_key=1 port_key=1 +" + +# Verify that existing check_uuid makes checkpatch happy. +try_checkpatch \ + "COMMON_PATCH_HEADER([tests/something.at]) + +check_uuid ovn-sbctl -- create FDB mac='"00\:00\:00\:00\:00\:03"' dp_key=1 port_key=1 + " + +# Verify that missing check for a get command does not cause checkpatch failures. +try_checkpatch \ + "COMMON_PATCH_HEADER([tests/something.at]) + +ovn-nbctl get Logical-Switch-Port p1 dynamic_addresses + " + +# Verify that missing check for a create command causes checkpatch failure - multiline. +try_checkpatch \ + "COMMON_PATCH_HEADER([tests/something.at]) + +ovn-sbctl \ + + -- create FDB mac='"00\:00\:00\:00\:00\:03"' dp_key=1 port_key=1 + " \ + "WARNING: ovn-sbctl return should be checked Consider adding check or check_uuid in front. + #8 FILE: tests/something.at:1: + ovn-sbctl + -- create FDB mac='00:00:00:00:00:03' dp_key=1 port_key=1 +" + +# Verify that missing check for a get command does not cause checkpatch failures - multiline. +try_checkpatch \ + "COMMON_PATCH_HEADER([tests/something.at]) + +ovn-nbctl \ + + -- get Logical-Switch-Port p1 dynamic_addresses + " + +# Verify that we can still store outout of ovn-nbctl w/o check. +try_checkpatch \ + "COMMON_PATCH_HEADER([tests/something.at]) + +lsp1_uuid=$(ovn-nbctl --bare --columns _uuid find Logical_Switch_Port name=lsp1) + " + +AT_CLEANUP diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index 35204daa29..a7360e47a2 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -203,6 +203,8 @@ def reset_counters(): __regex_nonascii_characters = re.compile("[^\u0000-\u007f]") __regex_efgrep = re.compile(r'.*[ef]grep.*$') __regex_hardcoded_table = re.compile(r'.*(table=[0-9]+)|.*(resubmit\(,[0-9]+\))') +__regex_ovn_nbctl = re.compile(r'^\s*ovn-nbctl ') +__regex_ovn_sbctl = re.compile(r'^\s*ovn-sbctl ') skip_leading_whitespace_check = False skip_trailing_whitespace_check = False @@ -377,6 +379,21 @@ def has_hardcoded_table(line): resubmit(,)""" return __regex_hardcoded_table.match(line) is not None +def has_nbctl_non_checked_commands(line): + """Return TRUE if the current line is missing check in front of ovn-nbctl, + except for commands containing 'find', 'show', 'list' and 'get'.""" + return (__regex_ovn_nbctl.match(line) is not None and + "find" not in line and "show" not in line and + "list" not in line and "get" not in line) + +def has_sbctl_non_checked_commands(line): + """Return TRUE if the current line is missing check in front of ovn-sbctl. + except for commands containing 'find', 'show', 'list' and 'get'.""" + return (__regex_ovn_sbctl.match(line) is not None and + "dump-flows" not in line and + "find" not in line and "show" not in line and + "list" not in line and "get" not in line) + def filter_comments(current_line, keep=False): """remove all of the c-style comments in a line""" STATE_NORMAL = 0 @@ -668,6 +685,18 @@ def empty_return(line): lambda: print_warning("Use of hardcoded table= or" " resubmit=(,) is discouraged in tests." " Consider using MACRO instead.")}, + + {'regex': r'\.at$', 'match_name': None, + 'check': lambda x: has_nbctl_non_checked_commands(x), + 'print': + lambda: print_warning("ovn-nbctl return should be checked" + " Consider adding check or check_uuid in front.")}, + + {'regex': r'\.at$', 'match_name': None, + 'check': lambda x: has_sbctl_non_checked_commands(x), + 'print': + lambda: print_warning("ovn-sbctl return should be checked" + " Consider adding check or check_uuid in front.")}, ] @@ -899,6 +928,7 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): reset_counters() + current_line = "" for line in text.split("\n"): if current_file != previous_file: previous_file = current_file @@ -906,6 +936,16 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): lineno = lineno + 1 total_line = total_line + 1 + if current_file.endswith(".at"): + if line.endswith("\\"): + current_line += line[:-1] + continue + else: + current_line += line + + line = current_line + current_line = "" + if line == "\f": # Form feed continue