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

From: Phil Carmody
Date: Sun Apr 15 2012 - 14:23:54 EST


On 22/03/12 17:22 +0100, ext Jiri Slaby wrote:
> 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?

I know I've already agreed to the above, as it makes perfect sense, but
I've just come across this, and it appears we're both wrong.

http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_317.htm
""""
...
void f(){}
...
Question 1: Does such a function definition give the function a type including a prototype for the rest of the translation unit?
...
Committee Response
The grammar states that an empty parens stands for an empty identifier list not an empty parameter-type-list.
The answer to question #1 is NO
"""

So it appears () is never sufficiently informative.
Phil
--
Phil Carmody
--
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/