Re: svn commit: r211228 - head/sys/kern

2010-08-13 Thread Bruce Evans

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


On Thursday 12 August 2010 12:37 pm, m...@freebsd.org wrote:

On Thu, Aug 12, 2010 at 9:13 AM, Jung-uk Kim j...@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___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r211228 - head/sys/kern

2010-08-12 Thread Jung-uk Kim
On Thursday 12 August 2010 12:37 pm, m...@freebsd.org wrote:
 On Thu, Aug 12, 2010 at 9:13 AM, Jung-uk Kim j...@freebsd.org 
wrote:
  Author: jkim
  Date: Thu Aug 12 16:13:24 2010
  New Revision: 211228
  URL: http://svn.freebsd.org/changeset/base/211228
 
  Log:
  �Provide description for 'machdep.disable_rtc_set' sysctl.
  �Clean up style(9) �nits. �Remove a redundant return statement
  and an unnecessary variable.

--- 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.

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.

Besides, I don't think it is revert worthy, either. ;-)

Thanks,

Jung-uk Kim
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org