On Fri, 14 Jan 2011 m...@freebsd.org wrote:

On Thu, Jan 13, 2011 at 9:56 PM, Bruce Evans <b...@optusnet.com.au> wrote:
Now with stricter type checking, even formats for integers are redundant.
The CTLTYPE now always matches the type, and the format should always
match the type. ?The 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>"="<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 output
  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
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to