Date: Tue, 22 Jul 2008 23:36:49 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: John Baldwin <jhb@FreeBSD.org> Cc: Tom Rhodes <trhodes@FreeBSD.org>, src-committers@FreeBSD.org, cvs-all@FreeBSD.org, cvs-src@FreeBSD.org Subject: Re: cvs commit: src/sys/netipx ipx_input.c ipx_usrreq.c Message-ID: <20080722224604.D17596@delplex.bde.org> In-Reply-To: <200807211100.16053.jhb@freebsd.org> References: <200807201525.m6KFPhoV014088@repoman.freebsd.org> <200807211100.16053.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 21 Jul 2008, John Baldwin wrote: > On Sunday 20 July 2008 11:25:20 am Tom Rhodes wrote: >> trhodes 2008-07-20 15:25:20 UTC >> >> FreeBSD src repository >> >> Modified files: >> sys/netipx ipx_input.c ipx_usrreq.c >> Log: >> SVN rev 180630 on 2008-07-20 15:25:20Z by trhodes >> >> Document a few sysctls. >> >> Reviewed by: rwatson > > FWIW, most other sysctls (though not all currently) start the first word with > uppercase and attempt to be complete sentences. This is consistent with > style(9)'s suggestions on code comments. Except most are not punctuated with a trailing ".". On ref8-i386.freebsd.org now, sysctl -da shows about 1470 sysctl with a non-null description, of which only 25 violate the non-punctuation rule, but almost half (713) violate the capitalization rule. A few of the ones with a trailing "." have multiple sentences on separate lines, but those are broken in other ways (the descriptions should be short, and must never be on separate lines, the latter so that simple greps work). Most of the violations of the capitalization rule are automatically generated for devices. sysctl -da output is spammed with the same descriptions for most devices of %desc, %driver, %location, %pnpinfo (all devices on i386 seem to have this though most have nothing to do with pnp) and %parent. Altogether there are 109 %desc's, 109 %driver's, 109 %location's, 109 %pnpinfo's and 143 %parents. This accounts for 579 of the 713 capitalization bugs. Please fix the style bugs of the source code of the sysctls when you fix these. The continuation indent is 4 chars (indent -ci4) in KNF but is mostly 11 chars (indent -lp) for these sysctls and is 12 chars (completely bogus) for at least one. Other recent additions of sysctl descriptions have much larger style bugs. The largest ones are in netipsec/ipsec.c. There the format of SYSCTL lines was highly non-KNF, but consistent, with excessive tabs to line things up. Now it is just ugly, with the description lines placed where they don't fit and almost always running beyond 80 columns. There is no way indent(1) can reproduce such fancy formatting of data declarations, especially when the data declarations are obfuscated in function-call-like macros, so such fancy formatting should rarely be used. indent(1) at least has a chance with normal SYSCTL formatting. A closer look shows that the fancy formatting in ipsec.c was already mostly wrong -- most things don't actually line up. In general, the first line is spit too early (normal is splitting after CTLFLAG*, but verbose name may prevent this) and the second line is not split enough. Splitting the first line too early almost guarantees that the second line will be too long unless it has a null description or is split. Then the tabs increase the mess. In other files, normal formatting of SYSCTL*() gives a chance of fitting the description on the scond line. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080722224604.D17596>