[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 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.
--

Re: [PATCH] x86: Merge x86_32 and x86_64 cpu_idle()

[H. Peter Anvin]

On 03/16/2012 01:18 PM, Richard Weinberger wrote:
> +/*
> + * The idle thread. There's no useful work to be
> + * done, so just try to conserve power and have a
> + * low exit latency (ie sit in a loop waiting for
> + * somebody to say that they'd like to reschedule)
> + */
> +void cpu_idle()

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

[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 --]