LinuxLists.cc - [RFC PATCH - diffstat only] include/net: Remove extern from function prototypes

2013-07-23 17:58:14

Subject: [RFC PATCH - diffstat only] include/net: Remove extern from function prototypes

2013-07-25 01:28:14

by Cong Wang

[permalink] [raw]

Subject: Re: [RFC PATCH - diffstat only] include/net: Remove extern from function prototypes

On Tue, 23 Jul 2013 at 17:58 GMT, Joe Perches <[email protected]> wrote:
> Function prototypes don't need to be declared
> extern in .h files. It's assumed by the compiler
> and is as unnecessary as using auto is when
> declaring automatic/local variables in a block.
>

Since we all know this, why bother it? Having "extern" doesn't harm
readability, instead it probably helps.

...
>
> I believe it makes grepping for extern useful as
> all the matches are actual variables and structs.
>

You need a semantic tool.

2013-07-25 01:41:25

by Joe Perches

[permalink] [raw]

Subject: Re: [RFC PATCH - diffstat only] include/net: Remove extern from function prototypes

On Thu, 2013-07-25 at 01:27 +0000, Cong Wang wrote:
> On Tue, 23 Jul 2013 at 17:58 GMT, Joe Perches <[email protected]> wrote:

You dropped the author from the cc list.
Always reply to the author.

> > Function prototypes don't need to be declared
> > extern in .h files. It's assumed by the compiler
> > and is as unnecessary as using auto is when
> > declaring automatic/local variables in a block.
> >
> Since we all know this, why bother it?

If everyone knew this, new ones wouldn't be added.
But a lot are.

Removing extern makes prototypes shorter and more readable.
More prototypes fit in a single line
More arguments fit on a multi-line prototypes.

> Having "extern" doesn't harm
> readability, instead it probably helps.

Sure, just like auto does. Why isn't that used?

> > I believe it makes grepping for extern useful as
> > all the matches are actual variables and structs.
> You need a semantic tool.

No, not really. I'm fairly comfortable with spatch.
spatch takes a long time to run for a lot of things.

2013-07-25 01:47:34

by Hannes Frederic Sowa

[permalink] [raw]

Subject: Re: [RFC PATCH - diffstat only] include/net: Remove extern from function prototypes

On Wed, Jul 24, 2013 at 06:41:20PM -0700, Joe Perches wrote:
> On Thu, 2013-07-25 at 01:27 +0000, Cong Wang wrote:
> > On Tue, 23 Jul 2013 at 17:58 GMT, Joe Perches <[email protected]> wrote:
>
> You dropped the author from the cc list.
> Always reply to the author.
>
> > > Function prototypes don't need to be declared
> > > extern in .h files. It's assumed by the compiler
> > > and is as unnecessary as using auto is when
> > > declaring automatic/local variables in a block.
> > >
> > Since we all know this, why bother it?
>
> If everyone knew this, new ones wouldn't be added.
> But a lot are.

Couldn't checkpatch take care of these?

Greetings,

Hannes

2013-07-25 02:59:14

by Joe Perches

[permalink] [raw]

Subject: [PATCH] checkpatch: Warn when using extern with function prototypes in .h files

Using the extern keyword on function prototypes is superfluous
visual noise so suggest removing it.

Using extern can cause unnecessary line wrapping at 80 columns
and unnecessarily long multi-line function prototypes.

Suggested-by: Hannes Frederic Sowa <[email protected]>
Signed-off-by: Joe Perches <[email protected]>
---
On Thu, 2013-07-25 at 03:47 +0200, Hannes Frederic Sowa wrote:
> On Wed, Jul 24, 2013 at 06:41:20PM -0700, Joe Perches wrote:
> > On Thu, 2013-07-25 at 01:27 +0000, Cong Wang wrote:
> > > On Tue, 23 Jul 2013 at 17:58 GMT, Joe Perches <[email protected]> wrote:
> > > > Function prototypes don't need to be declared
> > > > extern in .h files. It's assumed by the compiler
> > > > and is as unnecessary as using auto is when
> > > > declaring automatic/local variables in a block.
> > > Since we all know this, why bother it?
> > If everyone knew this, new ones wouldn't be added.
> > But a lot are.
> Couldn't checkpatch take care of these?

checkpatch doesn't work well for multiple lines so
this doesn't work on things like
extern unsigned long
foo(type bar);
but it does for the single line function prototypes.

Using --fix removes them.

scripts/checkpatch.pl | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 6918517..23126d4 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3866,6 +3866,16 @@ sub process {
}
}

+# check for new externs in .h files.
+ if ($realfile =~ /\.h$/ &&
+ $line =~ /^\+\s*(extern\s+)$Type\s*$Ident\s*\(/s) {
+ if (WARN("AVOID_EXTERNS",
+ "extern prototypes should be avoided in .h files\n" . $herecurr) &&
+ $fix) {
+ $fixed[$linenr - 1] =~ s/(.*)\bextern\b\s*(.*)/$1$2/;
+ }
+ }
+
# check for new externs in .c files.
if ($realfile =~ /\.c$/ && defined $stat &&
$stat =~ /^.\s*(?:extern\s+)?$Type\s+($Ident)(\s*)\(/s)
--
1.8.1.2.459.gbcd45b4.dirty

2013-07-30 07:10:04

by David Miller

[permalink] [raw]

Subject: Re: [RFC PATCH - diffstat only] include/net: Remove extern from function prototypes

From: Joe Perches <[email protected]>
Date: Tue, 23 Jul 2013 10:58:11 -0700

> Function prototypes don't need to be declared
> extern in .h files. It's assumed by the compiler
> and is as unnecessary as using auto is when
> declaring automatic/local variables in a block.
>
> I ran a script to remove these unnecessary extern
> uses from function prototypes in include/net/ as
> well as reflow the function arguments to 80 cols.
>
> It's 370KB, far too large to submit at once.
>
> Of course it could be broken into multiple patches
> for the various directories/subsystems.

No objections from me.

Please submit a small number at a time so that my already overflowing
backlog isn't overwhelmed even more.

Thanks.

2013-07-30 07:38:08

by Stephen Rothwell

[permalink] [raw]

Subject: Re: [RFC PATCH - diffstat only] include/net: Remove extern from function prototypes

Hi Joe,

On Tue, 30 Jul 2013 00:10:01 -0700 (PDT) David Miller <[email protected]> wrote:
>
> From: Joe Perches <[email protected]>
> Date: Tue, 23 Jul 2013 10:58:11 -0700
>
> > Function prototypes don't need to be declared
> > extern in .h files. It's assumed by the compiler
> > and is as unnecessary as using auto is when
> > declaring automatic/local variables in a block.
> >
> > I ran a script to remove these unnecessary extern
> > uses from function prototypes in include/net/ as
> > well as reflow the function arguments to 80 cols.
> >
> > It's 370KB, far too large to submit at once.
> >
> > Of course it could be broken into multiple patches
> > for the various directories/subsystems.
>
> No objections from me.

I must admit I prefer the other way (i.e. with the explicit extern). If
nothing else it makes it consistent with variable declarations in header
files (which need the extern) and "static inlines" ...

Making these churn changes also will almost certainly cause unnecessary
conflicts :-(

--
Cheers,
Stephen Rothwell [email protected]

Attachments:

(No filename) (1.06 kB)
(No filename) (836.00 B)
Download all attachments

2013-07-30 07:51:31

by David Miller

[permalink] [raw]

Subject: Re: [RFC PATCH - diffstat only] include/net: Remove extern from function prototypes

From: Stephen Rothwell <[email protected]>
Date: Tue, 30 Jul 2013 17:38:00 +1000

> I must admit I prefer the other way (i.e. with the explicit extern). If
> nothing else it makes it consistent with variable declarations in header
> files (which need the extern) and "static inlines" ...
>
> Making these churn changes also will almost certainly cause unnecessary
> conflicts :-(

Regardless of preferences, the whole tree is a mix, and we should choose
one way or the other consistently.

It seems like new code in other major subsystems are using the
no-extern scheme, and I'm more than happy to follow suit.

2013-08-01 12:21:16

by David Howells

[permalink] [raw]

Subject: Re: [PATCH] checkpatch: Warn when using extern with function prototypes in .h files

Joe Perches <[email protected]> wrote:

> Using the extern keyword on function prototypes is superfluous
> visual noise so suggest removing it.
>
> Using extern can cause unnecessary line wrapping at 80 columns
> and unnecessarily long multi-line function prototypes.
>
> Suggested-by: Hannes Frederic Sowa <[email protected]>
> Signed-off-by: Joe Perches <[email protected]>

Nak.

The "extern" makes it much easier to pick out at a glance.

Please standardise on there _being_ externs if you must do this.

It's gratuitous change anyway and not worth the churn.

David

2013-08-01 13:01:06

by Arend van Spriel

[permalink] [raw]

Subject: Re: [PATCH] checkpatch: Warn when using extern with function prototypes in .h files

On 08/01/2013 02:21 PM, David Howells wrote:
> Joe Perches <[email protected]> wrote:
>
>> Using the extern keyword on function prototypes is superfluous
>> visual noise so suggest removing it.
>>
>> Using extern can cause unnecessary line wrapping at 80 columns
>> and unnecessarily long multi-line function prototypes.
>>
>> Suggested-by: Hannes Frederic Sowa <[email protected]>
>> Signed-off-by: Joe Perches <[email protected]>
>
> Nak.
>
> The "extern" makes it much easier to pick out at a glance.

Really? Grouping function prototypes within the header file is much more
helpful for spotting them.

Regards,
Arend

> Please standardise on there _being_ externs if you must do this.
>
> It's gratuitous change anyway and not worth the churn.
>
> David
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>