svn commit: r211228 - head/sys/kern

Bruce Evans brde at optusnet.com.au
Fri Aug 13 10:50:14 UTC 2010


On Thu, 12 Aug 2010, Jung-uk Kim wrote:

> On Thursday 12 August 2010 12:37 pm, mdf at FreeBSD.org wrote:
>> On Thu, Aug 12, 2010 at 9:13 AM, Jung-uk Kim <jkim at freebsd.org>
> wrote:
>>> Log:
>>>  Provide description for 'machdep.disable_rtc_set' sysctl.
>>>  Clean up style(9)  nits.  Remove a redundant return statement
>>> and an unnecessary variable.

Some mailer mangled the whitespace, so the source code was unreadable and
had syntax errors (hard \xa0's) in it.  The hard \xa0's even got into
the plain text in the log message.

> --- >8 SNIP!!! --- >8 ---
>
>> While the device_printf() is a single statement, it spans multiple
>> lines.  It seems in this instance that clarity is preserved better
>> by using braces even though not required by C.
>>
>> Quoting style(9):
>>
>> Space after keywords (if, while, for, return, switch).  No braces
>> ('{' and '}') are used for control statements with zero or only a
>> single statement unless that statement is more than a single line
>> in which case they are permitted.
>>
>> So both styles are accepted, and I would think that changing this
>> is needless code churn.

The main style bug here is the use of C90 string concatenation to
obfuscate (not very long) messages.  Some of the messages fit on
1 line before device_printf() and syslogd add prefixes to them, but
the literal parts of them are misformatted across 3 lines in the
source code.  I try to keep messages shorted so that they fit in
1 line with normal indentation, or outdent long ones so that they
fit in 1 line with abnormal indentation.

> Some times it is very hard to read code when if statements contain
> excessive braces and 'else' statements although is permitted by
> style(9).  It was one of those cases.  Please note a 'else' block was
> also removed from there.

I agree, but don't mind the braces here.

Removing the else block makes the misformatting of a string across 3
lines of source more bogus than before, since after reducing the
indentation its lines are now split too early, and without splitting
too early the ugly split string would fit in only 2 lines.

I noticed 1 other unfixed style bug on a changed line: the SYSCTL_INT()
was split much earlier than necessary (before CTLFLAG*).  Now it is
split as late as possible (after the variable name, which is next after
CTLFLAG*).  But SYSCTL_INT() is conventionally always split after
CTLFLAG* unless this would give a too-long line.

Bruce


More information about the svn-src-all mailing list