mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mike Isely <isely@isely.net>
To: Roland Dreier <rdreier@cisco.com>
Cc: Hans Verkuil <hverkuil@xs4all.nl>,
	LKML <linux-kernel@vger.kernel.org>,
	v4l-dvb maintainer list <v4l-dvb-maintainer@linuxtv.org>
Subject: Re: CodingStyle question: multiple statements on a single line
Date: Tue, 2 Sep 2008 18:50:44 -0500 (CDT)	[thread overview]
Message-ID: <Pine.LNX.4.64.0809021830400.17907@cnc.isely.net> (raw)
In-Reply-To: <ada7i9ufa7x.fsf@cisco.com>

On Tue, 2 Sep 2008, Roland Dreier wrote:

>  > 2) No, never use the 'if (a) b;' construction. Put 'b;' on the next line 
>  > instead.
> 
> This is correct.  Always write simple if statements as
> 
> 	if (a)
> 		b;

Going back to the text:

<quote>
Don't put multiple statements on a single line unless you have
something to hide:
</quote>

Then what does "unless you have something to hide" refer to exactly?


<quote>
        if (condition) do_this;
          do_something_everytime;
</quote>

Realize that "if (condition) do_this;" is in fact one statement.  The 
"if (condition)" part is not something that can stand on its own; it is 
a predicate which is not completed without the rest of the statement.  
I interpret the example to be showing what is correct, not what is 
disallowed.  Both lines are single statements, each on their own line.  
I see the above as trying to illustrate outlawing of this sort of 
silliness:

        if (condition) do_this; do_something_everytime;

which will compile and run but is obviously misleading.

If anything the "do_this" is an example where one has something to hide, 
since it is not in the normal flow of execution (predicated instead by 
"if (condition)").

If anything,

	if (condition) do_this;

is safer than

	if (conditon)
		do_this;

because while in both cases it's one statement, the second case is split 
in exactly the spot where somebody's later errant line of code (say, a 
debug printk) will completely change what is going on.  With the whole 
statement on one line, this situation is avoided.  Or perhaps

	if (condition) {
		do_this;
	}

is another way to avoid the same issue, but that seems frowned upon for 
other reasons (below).

I know the answer to all this is just "but nobody does it that way".  
But my reading of the CodingStyle says this is allowed and that is what 
I thought was being asked - what does CodingStyle say?


> 
>  > And in general, why is this:
>  > 
>  > if (a) {
>  > 	b;
>  > }
>  > 
>  > not accepted by the CodingStyle? (At least as I understand it)
> 
> The braces take up another line of whitespace, which means less code
> fits on the screen.  And in simple cases, they don't add anything.
> Finally, the vast majority of the kernel leaves the braces off, so they
> look funny to people who read a lot of kernel code.

So by that reasoning "if (a) b;" - provided it stays under 80 columns - 
should be even better.  It occupies less space so that more code can fit 
on the screen.


> 
> And uniformity counts for a lot: most coding style rules are completely
> arbitrary, but having a uniform kernel style makes reading kernel code
> much easier.

What about drivers?  The statement has been made by others that there is 
a strong desire for outside drivers to be brought into mainline rather 
than being out-of-tree.  So must every chunk of code brought in this way 
be sanitized to this level of detail?  In many cases that can be a large 
(and some might say arbitrary) hurdle.


> 
> Keep in mind that common sense always trumps any mechanical rule.  So if
> there is some case where writing
> 
> 	if (a) {
> 		b;
> 	}
> 
> is clearly easier to read than leaving the braces off, then that would
> be OK.

That's great.  How does one reconcile this statement with subsystem 
maintainers who treat checkpatch.pl - which is the epitome of 
"mechanical rule" and has no notion of common sense - as the gatekeeper 
for all incoming changesets?

  -Mike


--

Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

  reply	other threads:[~2008-09-02 23:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-02 22:20 Hans Verkuil
2008-09-02 22:34 ` Roland Dreier
2008-09-02 23:50   ` Mike Isely [this message]
2008-09-03  0:23     ` Randy Dunlap
2008-09-03 12:19     ` Krzysztof Halasa
2008-09-03 18:38     ` Roland Dreier
2008-09-03 20:22       ` Lennart Sorensen
2008-09-03 12:15   ` Krzysztof Halasa
2008-09-03  6:57 ` Frans Meulenbroeks

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.64.0809021830400.17907@cnc.isely.net \
    --to=isely@isely.net \
    --cc=hverkuil@xs4all.nl \
    --cc=isely@pobox.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rdreier@cisco.com \
    --cc=v4l-dvb-maintainer@linuxtv.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®