linux-kernel - Re: [PATCH v3] checkpatch: add a rule to check general block comment style

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 16 Jul 2021 21:45:43 +0530 (IST)
From:   Ani Sinha <ani@...sinha.ca>
To:     Ani Sinha <ani@...sinha.ca>
cc:     linux-kernel@...r.kernel.org, anirban.sinha@...ia.com,
        mikelley@...rosoft.com, Andy Whitcroft <apw@...onical.com>,
        Joe Perches <joe@...ches.com>,
        Dwaipayan Ray <dwaipayanray1@...il.com>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>
Subject: Re: [PATCH v3] checkpatch: add a rule to check general block comment
 style

checkpatch maintainers, any comments?

On Wed, 14 Jul 2021, Ani Sinha wrote:

> The preferred style for long (multi-line) comments is:
>
> .. code-block:: c
>
> 	/*
> 	 * This is the preferred style for multi-line
> 	 * comments in the Linux kernel source code.
> 	 * Please use it consistently.
> 	 *
> 	 * Description:  A column of asterisks on the left side,
> 	 * with beginning and ending almost-blank lines.
> 	 */
>
> It seems rule in checkpatch.pl is missing to ensure this for
> non-networking related changes. This patch adds this rule.
>
> Tested with
> $ cat drivers/net/t.c
>     /* foo */
>
>     /*
>      * foo
>      */
>
>     /* foo
>      */
>
>     /* foo
>      * bar */
>
> $ ./scripts/checkpatch.pl -f drivers/net/t.c
> WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> line #1: FILE: drivers/net/t.c:1:
> +    /* foo */
>
> WARNING: networking block comments don't use an empty /* line, use /* Comment...
> line #4: FILE: drivers/net/t.c:4:
> +    /*
> +     * foo
>
> WARNING: Block comments use a trailing */ on a separate line
> line #11: FILE: drivers/net/t.c:11:
> +     * bar */
>
> total: 0 errors, 3 warnings, 0 checks, 11 lines checked
>
>
> For a non-networking related code we see the following when run for
> the same file:
>
> $ ./scripts/checkpatch.pl -f arch/x86/kernel/t.c
> WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> line #1: FILE: arch/x86/kernel/t.c:1:
> +    /* foo */
>
> WARNING: Block comments use a leading /* on a separate line
> line #7: FILE: arch/x86/kernel/t.c:7:
> +    /* foo
>
> WARNING: Block comments use a leading /* on a separate line
> line #10: FILE: arch/x86/kernel/t.c:10:
> +    /* foo
>
> WARNING: Block comments use a trailing */ on a separate line
> line #11: FILE: arch/x86/kernel/t.c:11:
> +     * bar */
>
> total: 0 errors, 4 warnings, 11 lines checked
>
> In the second case, there is no warning on line 4 and in the first
> case, there is no warning on line 10.
>
> Signed-off-by: Ani Sinha <ani@...sinha.ca>
> ---
>  scripts/checkpatch.pl | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> Changelog:
> v1: initial patch
> v2: commit log updated to reflect the output from checkpatch.pl
>     when run against the same file both in networking and
>     non-networking case. This helps in comparing results apples to
>     apples.
> v3: line numbers got lost in the commit log as git eliminated all lines
>     starting with '#'. Fixed it by prefixing with word 'line'. The work
>     'line' does not however appear in the checkpatch.pl output.
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 23697a6b1eaa..5f047b762aa1 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3833,6 +3833,14 @@ sub process {
>  			     "networking block comments don't use an empty /* line, use /* Comment...\n" . $hereprev);
>  		}
>
> +# Block comments use /* on a line of its own
> +		if (!($realfile =~ m@^(drivers/net/|net/)@) &&
> +		    $rawline !~ m@^\+.*/\*.*\*/[ \t)}]*$@ &&	#inline /*...*/
> +		    $rawline =~ m@^\+.*/\*\*?+[ \t]*[^ \t]@) { # /* or /** non-blank
> +		    WARN("BLOCK_COMMENT_STYLE",
> +			 "Block comments use a leading /* on a separate line\n" . $herecurr);
> +		}
> +
>  # Block comments use * on subsequent lines
>  		if ($prevline =~ /$;[ \t]*$/ &&			#ends in comment
>  		    $prevrawline =~ /^\+.*?\/\*/ &&		#starting /*
> --
> 2.25.1
>
>