Date: Sat, 15 Jan 2011 14:32:30 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: mdf@FreeBSD.org Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Bruce Evans <brde@optusnet.com.au> Subject: Re: svn commit: r217369 - in head/sys: cam/scsi sys Message-ID: <20110115134354.R16210@besplex.bde.org> In-Reply-To: <AANLkTimrxHnupc5kFU8%2BxBi0MC%2BbEiiUf9=aHMG22CM0@mail.gmail.com> References: <201101131820.p0DIKXip059402@svn.freebsd.org> <20110114162142.M27877@besplex.bde.org> <AANLkTimrxHnupc5kFU8%2BxBi0MC%2BbEiiUf9=aHMG22CM0@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --0-1911528158-1295062350=:16210 Content-Type: TEXT/PLAIN; charset=X-UNKNOWN; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE On Fri, 14 Jan 2011 mdf@FreeBSD.org wrote: > On Thu, Jan 13, 2011 at 9:56 PM, Bruce Evans <brde@optusnet.com.au> wrote= : >> Now with stricter type checking, even formats for integers are redundant= =2E >> The CTLTYPE now always matches the type, and the format should always >> match the type. =A0The space wasted for the format is 1 pointer plus the >> string. > > There are several issues here. First, for OPAQUE types the string is > interpreted by sysctl(8) to know how to interpret the binary blob. > Second, anyone can still use SYSCTL_OID(... CTLTYPE_INT, ..., "QU") > and there's not a lot that can be done to check it. Locally I have The format would be ignored for CTLTYPEs that describe an integer, and might be passed in a different way for SYSCTL_OID() (it might be attached to one of the other strings). > another patch for the various sysctl_handle_foo that asserts the > CTLTYPE_FOO matches the handler, but looking around the FreeBSD code > there's plenty of hand-rolled instances that won't pass; some of these > come from a SYSCTL_PROC setup. Perhaps we could start with a warning > message and upgrade to a KASSERT later. SYSCTL_PROC() and SYSCTL_OPAQUE() will always have little structure. I think the format string is meaningless and unused for SYSCTL_PROC(). For SYSCTL_OPAQUE(), it is used to encode the type (just by name). Only 5 types seem to be support, and there should be fewer: - T,dev_t: dev_t isn't important any more, and the kernel doesn't even generate any T,dev_t's for sysctl(8) to handle. - S,clockinfo: a historical mistake from 4.4BSD. It would be more useful to print the fields (all 4 of them) in this struct separately. Each sysctl took lots specialized of code in 4.4BSD, so it was easier to pack even non-closely related fields like the ones in clockinfo together. Even if you struct members together, it is convenient to have them separated in a subtree of a node for the struct. FreeBSD now has zillions more clock sysctls but doesn't pack any of the new ones into old or new structs. - S,timeval: might be useful, but it is used a whole once (for kern.boottime). - S,loadavg: from 4.4BSD. Like S,clockinfo, but not so bad. The fields belong together more. They can now be handled better as an array, like kern.cp_time[s] but unlike kern.clockrate. The only difference in sysctl(8) output would be removal of ornations for the struct (braces). The ornations make the output harder to filter, as do separate tokens, but separate tokens are natural for arrays. The field separator for arrays seems to be a space and that is better than a comma. For S_loadavg, the field separator is also a space and there are no field descriptors, but for S_clockinfo the ornations are very hard to parse -- there are commas in the field separators, and <name><space>"=3D"<space before each field. There should only be raw load average here, since load averages are presented better elsewhere. - S,vmtotal. A FreeBSD mistake. vmtotals are presented better elsewhere, e.g., in systat -v and vmstat. If not, then it should have been fixed there instead of bloating here. The multi-line ornately-formatted outpu= t generated by this messes up grepping for stuff. There are already rather too many sysctls for individual vmtotal and vmmeter fields. > Thirdly, at the moment sysctl(8) doesn't fetch the access flags and > type specifier and only looks at the string. This is also fixable. It seems to use CTLTYPE* for parsing user values only. Is `kind' fetched for writing (to the kernel) but not for reading? Bruce --0-1911528158-1295062350=:16210--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110115134354.R16210>