On 27 Mar 2025, at 16:10, Aaron Conole wrote:
> Checkpatch has the ability to scan source files rather than
> exclusively scanning patch files. In this mode, some checks
> get disabled (ex: all of the header and commit message
> parsing) and the display modes are adjusted to simply print
> the file and line, rather than including the patch line
> details.
>
> When checkpatch was updated for subject parsing, it was
> inadvertently ignoring the source file mode. This means
> any checks run against a source file will always print a
> bogus subject suggestion, and warning. Fix this by not
> printing these warnings when in source file mode. This
> should have no effect on checkpatch against actual patch
> files.
>
> Additionally, introduce two simple tests to verify that
> the source file mode still functions.
>
> Fixes: 1b8fa4a66aa4 ("checkpatch: Add checks for the subject line.")
> Signed-off-by: Aaron Conole <[email protected]>
The patch looks good to me with one small comment below. If applied during
commit, it looks good to me.
Acked-by: Eelco Chaudron <[email protected]>
> ---
> v3: Address Eelco's feedback
> v2: Added unit tests to make sure we have some basic checks
> for source file mode.
>
> tests/checkpatch.at | 69 +++++++++++++++++++++++++++++++++++++++++
> utilities/checkpatch.py | 8 +++--
> 2 files changed, 74 insertions(+), 3 deletions(-)
>
> diff --git a/tests/checkpatch.at b/tests/checkpatch.at
> index 2ed2ec878b..29a25fef44 100755
> --- a/tests/checkpatch.at
> +++ b/tests/checkpatch.at
> @@ -38,6 +38,31 @@ Subject: Patch this is.
> $top_srcdir/utilities/checkpatch.py $3 -q test.patch])
> fi
> }
> +
> +# try_checkpatch_c_file FILE [ERRORS] [CHECKPATCH-ARGS]
> +#
> +# Runs checkpatch against test FILE expecting the set of specified
Should FILE not be FILE_CONTENTS also, there is a double space after FILE.
> +# ERRORS (and warnings). Optionally, sets [CHECKPATCH-ARGS]
> +try_checkpatch_c_file() {
> + echo "$1" | sed 's/^ //' > test.c
> +
> + # Take expected output from $2.
> + if test -n "$2"; then
> + echo "$2" | sed 's/^ //' > expout
> + else
> + : > expout
> + fi
> +
> + if test -s expout; then
> + AT_CHECK([OVS_SRC_DIR=$top_srcdir $PYTHON3 \
> + $top_srcdir/utilities/checkpatch.py $3 -q -f test.c],
> + [1], [stdout])
> + AT_CHECK([sed '/^Lines checked:/,$d' stdout], [0], [expout])
> + else
> + AT_CHECK([OVS_SRC_DIR=$top_srcdir $PYTHON3 \
> + $top_srcdir/utilities/checkpatch.py $3 -q -f test.c])
> + fi
> +}
> OVS_END_SHELL_HELPERS
>
> AT_SETUP([checkpatch - sign-offs])
> @@ -657,3 +682,47 @@ try_checkpatch \
> "-a"
>
> AT_CLEANUP
> +
> +AT_SETUP([checkpatch - file contents checks - bare return])
> +try_checkpatch_c_file \
> + "#include <foo.h>
> + #include <bar.h>
> +
> + void foo() {
> + return;
> + }" \
> + "WARNING: Empty return followed by brace, consider omitting
> + test.c:6:
> + }
> + "
> +AT_CLEANUP
> +
> +AT_SETUP([checkpatch - file contents checks - parenthesized constructs])
> +
> +for ctr in 'if' 'while' 'switch' 'HMAP_FOR_EACH' 'BITMAP_FOR_EACH_1'; do
> +try_checkpatch_c_file \
> + "#include <foo.h>
> + #include <bar.h>
> +
> + void foo() {
> + $ctr (check_node) {
> + something(check_node);
> + }
> + }
> + "
> +
> +try_checkpatch_c_file \
> + "#include <foo.h>
> + #include <bar.h>
> +
> + void foo() {
> + $ctr ( first_run) {
> + something(check_node);
> + }
> + }" \
> + "ERROR: Improper whitespace around control block
> + test.c:5:
> + $ctr ( first_run) {
> + "
> +done
> +AT_CLEANUP
> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
> index 3c09b94cf5..b356731ea4 100755
> --- a/utilities/checkpatch.py
> +++ b/utilities/checkpatch.py
> @@ -1189,10 +1189,12 @@ def ovs_checkpatch_file(filename):
> else:
> mail.add_header('Subject', sys.argv[-1])
>
> - print("Subject missing! Your provisional subject is",
> - mail['Subject'])
> + if not checking_file:
> + print("Subject missing! Your provisional subject is",
> + mail['Subject'])
>
> - if run_subject_checks('Subject: ' + mail['Subject'], spellcheck):
> + if not checking_file and run_subject_checks('Subject: ' +
> mail['Subject'],
> + spellcheck):
> result = True
>
> ovs_checkpatch_print_result()
> --
> 2.47.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
