Mark Brown wrote:
> No substantial changes, this just tidies up a bunch of coding style
> issues that ought to be fixed up before merge (which I'll do when the
> GTA02 machine support is queued for merge).
Very nice, thanks ! One question, though:
> - if(val) {
> + if (val)
> lm4853_state |= LM4853_AMP;
> - } else {
> + else
> lm4853_state &= ~LM4853_AMP;
> - }
This appears to be in contradiction with the following passage of
Documentation/CodingStyle:
| Do not unnecessarily use braces where a single statement will do.
|
| if (condition)
| action();
|
| This does not apply if one branch of a conditional statement is a single
| statement. Use braces in both branches.
|
| if (condition) {
| do_this();
| do_that();
| } else {
| otherwise();
| }
I must say that I don't particularly like this rule. I'd rather write
if (foo)
then_this();
else
otherwise_that();
(the else is easy to spot) and even
if (foo)
then_this();
else {
otherwise();
that();
}
(still no problem spotting the else) but never
if (foo) {
then();
this();
} else
otherwise_that();
In the latter case, I'd try to change the logic such that it becomes
either a one-statement if-true branch, or something else entirely.
What's your interpretation of the scripture ?
- Werner
