linux-kernel - Re: [PATCH 1/5] checkpatch: Add tests for function pointer style misuses

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]

Message-ID: <20131226200723.GA7267@leaf>
Date:	Thu, 26 Dec 2013 12:07:23 -0800
From:	Josh Triplett <josh@...htriplett.org>
To:	Joe Perches <joe@...ches.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Manfred Spraul <manfred@...orfullife.com>,
	Andy Whitcroft <apw@...onical.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/5] checkpatch: Add tests for function pointer style
 misuses

On Thu, Dec 26, 2013 at 11:20:28AM -0800, Joe Perches wrote:
> Kernel style uses function pointers in this form:
>         "type (*funcptr)(args...)"
> 
> Emit warnings when this function pointer form isn't used.
> 
> Signed-off-by: Joe Perches <joe@...ches.com>

Two comments below.

>  scripts/checkpatch.pl | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 0fcbc5f..f5d4560 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2810,6 +2810,65 @@ sub process {
>  			}
>  		}
>  
> +# Function pointer declarations
> +# check spacing between type, funcptr, and args
> +# canonical declaration is "type (*funcptr)(args...)"
> +#
> +# the $Declare variable will capture all spaces after the type
> +# so check it for multiple spaces
> +		if ($line =~ /^.\s*($Declare)\((\s*)\*(\s*)$Ident(\s*)\)(\s*)\(/) {
> +			my $declare = $1;
> +			my $pre_pointer_space = $2;
> +			my $post_pointer_space = $3;
> +			my $funcname = $4;
> +			my $post_funcname_space = $5;
> +			my $pre_args_space = $6;
> +
> +			if ($declare !~ /\s$/) {
> +				WARN("SPACING",
> +				     "missing space after return type\n" . $herecurr);
> +			}
> +
> +# unnecessary space "type  (*funcptr)(args...)"
> +			elsif ($declare =~ /\s{2,}$/) {
> +				WARN("SPACING",
> +				     "Multiple spaces after return type\n" . $herecurr);
> +			}
> +
> +# unnecessary space "type ( *funcptr)(args...)"
> +			if (defined $pre_pointer_space &&
> +			    $pre_pointer_space =~ /^\s/) {
> +				WARN("SPACING",
> +				     "Unnecessary space after function pointer open parenthesis\n" . $herecurr);

There are two parentheticals in a proper function pointer declaration,
so this is slightly ambiguous.  Perhaps "between open parenthesis and
'*' in function pointer declaration"?

> +# unnecessary space "type (* funcptr)(args...)"
> +			if (defined $post_pointer_space &&
> +			    $post_pointer_space =~ /^\s/) {
> +				WARN("SPACING",
> +				     "Unnecessary space before function pointer name\n" . $herecurr);
> +			}
> +
> +# unnecessary space "type (*funcptr )(args...)"
> +			if (defined $post_funcname_space &&
> +			    $post_funcname_space =~ /^\s/) {
> +				WARN("SPACING",
> +				     "Unnecessary space after function pointer name\n" . $herecurr);
> +			}
> +
> +# unnecessary space "type (*funcptr) (args...)"
> +			if (defined $pre_args_space &&
> +			    $pre_args_space =~ /^\s/) {
> +				WARN("SPACING",
> +				     "Unnecessary space before function pointer name\n" . $herecurr);

Copy/paste problem here?  I think this should be "Unnecessary space
before function pointer argument types" or similar.

With both of the above two issues fixed:

Reviewed-by: Josh Triplett <josh@...htriplett.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/