[PATCH 0/30] return statement cleanup - kill pointless parentheses

[PATCH 0/30] return statement cleanup - kill pointless parentheses

[Jesper Juhl]

Ok, here's the first batch of return statement cleanups (*many* to go).

The following patches clean up return statements of the forms
	return(foo);
	return ( fn() );
	return (123);
	return(a + b);
etc. To be instead
	return foo;
	return fn();
	return 123
	return a + b;

There are rare cases where a return expression is long and/or complicated 
where having the parentheses is a nice readability help. In those cases 
I've let them be.

This is only the first batch. I have many, many more patches lined up, but 
I need sleep as well as the next erson, so I'm going to submit this in 
batches over a couple of days (unless someone yells STOP ofcourse ;)

If these patches are generally acceted then I think it would make sense to 
make a small addition to Documentation/CodingStyle mentioning the prefered 
form of return statements, so we (hopefully) won't have to do cleanups 
like this too often in the future.
Below I've included a proposed patch adding such a bit to CodingStyle.

As I mentioned in my previous mail to lkml with the subject of 
"[RFC][example patch inside] return statement cleanups, get rid of unnecessary parentheses"
The reasons for doing this are :

1) the parentheses are pointless.
2) removing the parentheses decreases source file size slightly.
3) they look odd and when reading code you don't want to be stopped wondering - no parentheses is simply more readable (at least to me).
4) When I've submitted patches for other stuff in the past I've a few times been asked if I could fix up the return statements while I was at it - so it seems to be wanted.

Comments are welcome - as always :)


Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>

--- linux-2.6.10-rc3-bk8-orig/Documentation/CodingStyle	2004-10-18 23:53:46.000000000 +0200
+++ linux-2.6.10-rc3-bk8/Documentation/CodingStyle	2004-12-16 00:28:37.000000000 +0100
@@ -183,7 +183,28 @@
 to understand what you did 2 weeks from now.
 
 
-		Chapter 6: Centralized exiting of functions
+		Chapter 6: return statements and parentheses
+
+'return' is not a function, it's a statement.
+A lot of people like to write return(n); or return(fn()); or similar. Well
+don't. Adding parentheses to return is pointless in 99+% of all cases, it
+gains you nothing at all, except people spending more time puzzling over
+your code. 
+There are rare excetions where the return statement is a long complicated 
+expression and the parentheses may help to improve readability, but the 
+general rule is to avoid them unless there's a really good reason.
+
+so, don't write:
+	return(foo);
+	return(fn());
+	return ( 123456 );
+or similar. But do write:
+	return foo;
+	return fn();
+	return 123456;
+
+
+		Chapter 7: Centralized exiting of functions
 
 Albeit deprecated by some people, the equivalent of the goto statement is
 used frequently by compilers in form of the unconditional jump instruction.
@@ -220,7 +241,7 @@
 	return result;
 }
 
-		Chapter 7: Commenting
+		Chapter 8: Commenting
 
 Comments are good, but there is also a danger of over-commenting.  NEVER
 try to explain HOW your code works in a comment: it's much better to
@@ -237,7 +258,7 @@
 it.
 
 
-		Chapter 8: You've made a mess of it
+		Chapter 9: You've made a mess of it
 
 That's OK, we all do.  You've probably been told by your long-time Unix
 user helper that "GNU emacs" automatically formats the C sources for
@@ -285,7 +306,7 @@
 remember: "indent" is not a fix for bad programming.
 
 
-		Chapter 9: Configuration-files
+		Chapter 10: Configuration-files
 
 For configuration options (arch/xxx/Kconfig, and all the Kconfig files),
 somewhat different indentation is used.
@@ -310,7 +331,7 @@
 experimental options should be denoted (EXPERIMENTAL).
 
 
-		Chapter 10: Data structures
+		Chapter 11: Data structures
 
 Data structures that have visibility outside the single-threaded
 environment they are created and destroyed in should always have
@@ -341,7 +362,7 @@
 have a reference count on it, you almost certainly have a bug.
 
 
-		Chapter 11: Macros, Enums, Inline functions and RTL
+		Chapter 12: Macros, Enums, Inline functions and RTL
 
 Names of macros defining constants and labels in enums are capitalized.
 
@@ -396,7 +417,7 @@
 covers RTL which is used frequently with assembly language in the kernel.
 
 
-		Chapter 12: Printing kernel messages
+		Chapter 13: Printing kernel messages
 
 Kernel developers like to be seen as literate. Do mind the spelling
 of kernel messages to make a good impression. Do not use crippled
@@ -407,7 +428,7 @@
 Printing numbers in parentheses (%d) adds no value and should be avoided.
 
 
-		Chapter 13: References
+		Chapter 14: References
 
 The C Programming Language, Second Edition
 by Brian W. Kernighan and Dennis M. Ritchie.



PS.
Andrew: I'm CC'ing you on this first mail since you are 2.6 maintainer, 
and if you NACK these patches I won't bother with more, but don't worry, I 
won't spam your inbox with all 30 , I trust you can pick them up from lkml 
if wanted.

Re: [PATCH 0/30] return statement cleanup - kill pointless parentheses

[Pavel Machek]

Hi!

> Ok, here's the first batch of return statement cleanups (*many* to go).
> 
> The following patches clean up return statements of the forms
> 	return(foo);
> 	return ( fn() );
> 	return (123);
> 	return(a + b);
> etc. To be instead
> 	return foo;
> 	return fn();
> 	return 123
> 	return a + b;

Yes, I like that. Please also go through drivers/acpi -- there's lot
of annoying parenthesis there ;-).
								Pavel

-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: [PATCH 0/30] return statement cleanup - kill pointless parentheses

[Jesper Juhl]

On Thu, 16 Dec 2004, Pavel Machek wrote:

> Hi!
> 
> > Ok, here's the first batch of return statement cleanups (*many* to go).
> > 
> > The following patches clean up return statements of the forms
> > 	return(foo);
> > 	return ( fn() );
> > 	return (123);
> > 	return(a + b);
> > etc. To be instead
> > 	return foo;
> > 	return fn();
> > 	return 123
> > 	return a + b;
> 
> Yes, I like that. Please also go through drivers/acpi -- there's lot
> of annoying parenthesis there ;-).
> 								Pavel

Sure thing, I'll add that to my must_do list :)
There seems to be very mixed feelings about this, so I think what I'll do 
is to pick a few files from different dirs and submit patches for them, if 
I then get positive feedback I'll do more in that dir, if not I'll just 
leave it.
Thank you for responding. 

-- 
Jesper Juhl

Re: [PATCH 0/30] return statement cleanup - kill pointless parentheses

[Arnd Bergmann]

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

On Dunnersdag 16 Dezember 2004 01:02, Jesper Juhl wrote:
> If these patches are generally acceted then I think it would make sense to 
> make a small addition to Documentation/CodingStyle mentioning the prefered 
> form of return statements, so we (hopefully) won't have to do cleanups 
> like this too often in the future.
> Below I've included a proposed patch adding such a bit to CodingStyle.

I think the change in Documentation/CodingStyle is useful, even though I
don't really like changing all the existing code without going through
the respective maintainers.

> 1) the parentheses are pointless.
> 2) removing the parentheses decreases source file size slightly.
> 3) they look odd and when reading code you don't want to be stopped wondering
>    - no parentheses is simply more readable (at least to me). 
> 4) When I've submitted patches for other stuff in the past I've a few times
>    been asked if I could fix up the return statements while I was at it - so
>    it seems to be wanted.  

This is basically the same category as the first three chapters of
CodingStyle. It's not nice to read, but there is no real problem in the 
code. Think of these issues as whitespace fixes: you are making the job
harder for code maintainers for very little gain. I would suggest that
you submit these patches only to the code maintainers, not to the Trivial
Patch Monkey or Andrew.

Or even better, change scripts/Lindent to do the change automatically for
code that it is used on, if that can be done in a reliable way.

 Arnd <><



[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 0/30] return statement cleanup - kill pointless parent heses

[Jesper Juhl]

Arnd Bergmann wrote:
> On Dunnersdag 16 Dezember 2004 01:02, Jesper Juhl wrote:
> 
>>If these patches are generally acceted then I think it would make
> 
> sense to 
> 
>>make a small addition to Documentation/CodingStyle mentioning the
> 
> prefered 
> 
>>form of return statements, so we (hopefully) won't have to do cleanups
> 
> 
>>like this too often in the future.
>>Below I've included a proposed patch adding such a bit to CodingStyle.
> 
> I think the change in Documentation/CodingStyle is useful, even though I
> don't really like changing all the existing code without going through
> the respective maintainers.
> 
I guess you have a point. I won't submit more of these through LKML but 
will stick to working with maintainers.
These first 30 patches were mostly meant to "test the waters".

I'm glad you like the addition to CodingStyle. :)

> 
> This is basically the same category as the first three chapters of
> CodingStyle. It's not nice to read, but there is no real problem in the 
> code. Think of these issues as whitespace fixes: you are making the job
> harder for code maintainers for very little gain. I would suggest that
> you submit these patches only to the code maintainers, not to the
> Trivial
> Patch Monkey or Andrew.

Will do.


> Or even better, change scripts/Lindent to do the change automatically
> for
> code that it is used on, if that can be done in a reliable way.
>  Arnd <><

I've never actually looked at how that script does its work, guess now's 
a good time to start looking :)


-- 
Jesper Juhl