Re: [PATCH 1/5] checkpatch: Add tests for function pointer stylemisuses

[Josh Triplett]

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@xxxxxxxxxxx>

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@xxxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/