[PATCH] checkpatch.pl: Check for functions without a real prototype

[PATCH] checkpatch.pl: Check for functions without a real prototype

[Richard Weinberger]

Functions like this one are evil:

void foo()
{
	...
}

Signed-off-by: Richard Weinberger <richard@nod.at>
CC: hpa@zytor.com
---
 scripts/checkpatch.pl |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a3b9782..acc5e0f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2304,6 +2304,19 @@ sub process {
 			      "open brace '{' following function declarations go on the next line\n" . $herecurr);
 		}
 
+		if ($line=~/$Type\s*$Ident\(\)/) {
+			ERROR("FUNCTION_NO_PROTOTYPE",
+"Function without a real prototype\n" . $herecurr .
+"Thou shalt not, in the language of C, under any circumstances, on the
+pain of death, declare or define a function with an empty set of
+parentheses, for though in the language of C++ it meaneth the same as
+(void), in C it meaneth (...) which is of meaningless as there be no
+anchor argument by which the types of the varadic arguments can be
+expressed, and which misleadeth the compiler into allowing unsavory code
+and in some cases generate really ugly stuff for varadic handling.
+	-hpa\n");
+		}
+
 # open braces for enum, union and struct go on the same line.
 		if ($line =~ /^.\s*{/ &&
 		    $prevline =~ /^.\s*(?:typedef\s+)?(enum|union|struct)(?:\s+$Ident)?\s*$/) {
-- 
1.7.7.3

Re: [PATCH] checkpatch.pl: Check for functions without a real prototype

[Joe Perches]

On Sat, 2012-03-17 at 00:04 +0100, Richard Weinberger wrote:
> Functions like this one are evil:
> void foo()
> {
> 	...
> }
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -2304,6 +2304,19 @@ sub process {
[]
> +		if ($line=~/$Type\s*$Ident\(\)/) {

Perhaps this should be:
		if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) {

> +			ERROR("FUNCTION_NO_PROTOTYPE",
> +"Function without a real prototype\n" . $herecurr .

Sensible, but maybe:
"Bad function definition - $1() should probably be $1(void)\n"

> +"Thou shalt not, in the language of C, under any circumstances, on the
> +pain of death, declare or define a function with an empty set of
> +parentheses, for though in the language of C++ it meaneth the same as
> +(void), in C it meaneth (...) which is of meaningless as there be no
> +anchor argument by which the types of the varadic arguments can be
> +expressed, and which misleadeth the compiler into allowing unsavory code
> +and in some cases generate really ugly stuff for varadic handling.
> +	-hpa\n");

Humorous once, painful in twice, annoying after.

cheers,  Joe

Re: [PATCH] checkpatch.pl: Check for functions without a real prototype

[Richard Weinberger]

[-- Attachment #1: Type: text/plain, Size: 999 bytes --]

Am 17.03.2012 00:31, schrieb Joe Perches:
> Perhaps this should be:
> 		if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) {

Makes sense.

>> +			ERROR("FUNCTION_NO_PROTOTYPE",
>> +"Function without a real prototype\n" . $herecurr .
> 
> Sensible, but maybe:
> "Bad function definition - $1() should probably be $1(void)\n"

Too.

>> +"Thou shalt not, in the language of C, under any circumstances, on the
>> +pain of death, declare or define a function with an empty set of
>> +parentheses, for though in the language of C++ it meaneth the same as
>> +(void), in C it meaneth (...) which is of meaningless as there be no
>> +anchor argument by which the types of the varadic arguments can be
>> +expressed, and which misleadeth the compiler into allowing unsavory code
>> +and in some cases generate really ugly stuff for varadic handling.
>> +	-hpa\n");
> 
> Humorous once, painful in twice, annoying after.
> 

It has to be annoying and painful. :-)

Thanks,
//richard


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[PATCH] checkpatch.pl: Check for functions without a real prototype

[Richard Weinberger]

Functions like this one are evil:

void foo()
{
	...
}

Signed-off-by: Richard Weinberger <richard@nod.at>
CC: hpa@zytor.com
---
 scripts/checkpatch.pl | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 61090e0..a1b846d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2785,6 +2785,19 @@ sub process {
 			      "open brace '{' following function declarations go on the next line\n" . $herecurr);
 		}
 
+		if ($line=~/$Type\s*$Ident\(\)/) {
+			ERROR("FUNCTION_NO_PROTOTYPE",
+"Function without a real prototype\n" . $herecurr .
+"Thou shalt not, in the language of C, under any circumstances, on the
+pain of death, declare or define a function with an empty set of
+parentheses, for though in the language of C++ it meaneth the same as
+(void), in C it meaneth (...) which is of meaningless as there be no
+anchor argument by which the types of the varadic arguments can be
+expressed, and which misleadeth the compiler into allowing unsavory code
+and in some cases generate really ugly stuff for varadic handling.
+	-hpa\n");
+		}
+
 # open braces for enum, union and struct go on the same line.
 		if ($line =~ /^.\s*{/ &&
 		    $prevline =~ /^.\s*(?:typedef\s+)?(enum|union|struct)(?:\s+$Ident)?\s*$/) {
-- 
1.8.4.2

Re: [PATCH] checkpatch.pl: Check for functions without a real prototype

[Joe Perches]

On Mon, 2013-11-18 at 23:31 +0100, Richard Weinberger wrote:
> Functions like this one are evil:
> 
> void foo()
> {
> 	...
> }
> 
> Signed-off-by: Richard Weinberger <richard@nod.at>
> CC: hpa@zytor.com

I won't take simple resubmissions without updating
based on the notes I gave you.

And now, this should be updated to allow --fix use too.

I'll take care of it instead.

Re: [PATCH] checkpatch.pl: Check for functions without a real prototype

[Richard Weinberger]

Am Montag, 18. November 2013, 14:36:22 schrieb Joe Perches:
> On Mon, 2013-11-18 at 23:31 +0100, Richard Weinberger wrote:
> > Functions like this one are evil:
> > 
> > void foo()
> > {
> > 
> > 	...
> > 
> > }
> > 
> > Signed-off-by: Richard Weinberger <richard@nod.at>
> > CC: hpa@zytor.com
> 
> I won't take simple resubmissions without updating
> based on the notes I gave you.

Of course. I managed it to send the *old* patch file.
Will resend in a minute...

> And now, this should be updated to allow --fix use too.
> 
> I'll take care of it instead.

Thanks,
//richard

[PATCH] checkpatch.pl: Check for function declarations without arguments

[Joe Perches]

Functions like this one are evil:

void foo()
{
	...
}

Because these functions allow variadic arguments without
checking the arguments at all.
 
Original-patch-by: Richard Weinberger <richard@nod.at>
Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 61090e0..2e1ff0c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2634,6 +2634,15 @@ sub process {
 				$herecurr);
                }
 
+# check for function declarations without arguments like "int foo()"
+		if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) {
+			if (ERROR("FUNCTION_WITHOUT_ARGS",
+				  "Bad function definition - $1() should probably be $1(void)\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~ s/(\b($Type)\s+($Ident))\s*\(\s*\)/$2 $3(void)/;
+			}
+		}
+
 # check for declarations of struct pci_device_id
 		if ($line =~ /\bstruct\s+pci_device_id\s+\w+\s*\[\s*\]\s*\=\s*\{/) {
 			WARN("DEFINE_PCI_DEVICE_TABLE",

Re: [PATCH] checkpatch.pl: Check for function declarations without arguments

[Borislav Petkov]

On Mon, Nov 18, 2013 at 03:23:01PM -0800, Joe Perches wrote:
> Functions like this one are evil:
> 
> void foo()
> {
> 	...
> }
> 
> Because these functions allow variadic arguments without
> checking the arguments at all.
>  
> Original-patch-by: Richard Weinberger <richard@nod.at>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  scripts/checkpatch.pl | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 61090e0..2e1ff0c 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2634,6 +2634,15 @@ sub process {
>  				$herecurr);
>                 }
>  
> +# check for function declarations without arguments like "int foo()"
> +		if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) {
> +			if (ERROR("FUNCTION_WITHOUT_ARGS",
> +				  "Bad function definition - $1() should probably be $1(void)\n" . $herecurr) &&

I liked the nice preaching comment better - this one is boring.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--