Re: [PATCH 1/1] checkpatch.pl: thou shalt not use () or (...) infunction declarations

From: Jiri Slaby
Date: Thu Mar 22 2012 - 12:22:39 EST


On 03/22/2012 04:27 PM, Phil Carmody wrote:
> After HPA's wonderful lkml post, referenced, it seems worth trying to
> detect this robomatically.
>
> Signed-off-by: Phil Carmody <ext-phil.2.carmody@xxxxxxxxx>
> ---
> scripts/checkpatch.pl | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index a3b9782..3993011 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1881,6 +1881,10 @@ sub process {
> substr($ctx, 0, $name_len + 1, '');
> $ctx =~ s/\)[^\)]*$//;
>
> + if ($ctx =~ /^\s*(?:\.\.\.)?\s*$/) {
> + # HPA explains why: http://lwn.net/Articles/487493/
> + ERROR("(...) and () are not sufficiently informative function declarations\n$hereline");
> + }

That explanation is not fully correct. C99 explicitly says (6.7.5.3.14):
An identifier list declares only the identifiers of the parameters of
the function. An empty list in a function declarator that is part of a
definition of that function specifies that the function has no
parameters. The empty list in a function declarator that is not part of
a definition of that function specifies that no information about the
number or types of the parameters is supplied.

So what you are trying to force here holds only for (forward)
declarations. Not for functions with definitions (bodies). Is checkpatch
capable to differ between those?

thanks,
-- js suse labs

--
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/