On Thu, Jan 13, 2011 at 9:56 PM, Bruce Evans <b...@optusnet.com.au> wrote: > On Thu, 13 Jan 2011, Matthew D Fleming wrote: > >> Log: >> Add a 64-bit hex-printed sysctl(9) since there is at least one place in >> the code that wanted it. It is named X64 rather than XQUAD since the >> quad name is a historical abomination that should not be perpetuated. > > :-). It is only long long that is abominable. Both are historical. > > I think these X formats shouldn't exist, even as defaults. Instead, > sysctl(8) should have option(s) to control the format. I'm used to > typing lots of "p/x"s to get hex formatting in gdb. There is little > reason for sysctl(8) to have finer-grained control than gdb (sysctl > now has hard-coded defaults instead of control). > > 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 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. 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. I do agree that the incredibly limited use of XINT, XLONG, and the one use of X64 mean that it's not a widely used feature, and can probably be better done with a -x flag to sysctl(8). Most bitflags are still added as SYSCTL_UINT, so one has to re-interpret the value anyways to see which bits are set. > Perhaps there are some remaining type errors involving the kernel type > being signed when it should be unsigned, or vice versa. This could > be "fixed" by mis-specifying the format with a U, or vice versa. Since > the specification is usually done by invoking a SYSCTL*() macro, most > such fixes, if any, would have been undone by changing the macro to > match the type, and it would take fixing the kernel type to fix the > userland format. The framework is in place to fix the obvious type errors for scalar types, but I haven't yet finished making the kernel compile cleanly in universe mode. Until then the check is still if 0'd out. Thanks, matthew > >> Modified: head/sys/cam/scsi/scsi_da.c >> >> ============================================================================== >> --- head/sys/cam/scsi/scsi_da.c Thu Jan 13 18:20:27 2011 (r217368) >> +++ head/sys/cam/scsi/scsi_da.c Thu Jan 13 18:20:33 2011 (r217369) >> @@ -1127,9 +1127,9 @@ dasysctlinit(void *context, int pending) >> struct ccb_trans_settings_fc *fc = &cts.xport_specific.fc; >> if (fc->valid & CTS_FC_VALID_WWPN) { >> softc->wwpn = fc->wwpn; >> - SYSCTL_ADD_XLONG(&softc->sysctl_ctx, >> + SYSCTL_ADD_X64(&softc->sysctl_ctx, >> SYSCTL_CHILDREN(softc->sysctl_tree), >> - OID_AUTO, "wwpn", CTLTYPE_QUAD | CTLFLAG_RD, >> + OID_AUTO, "wwpn", CTLFLAG_RD, >> &softc->wwpn, "World Wide Port Name"); >> } >> } >> > > Hmm, forcing hex might be best for flags (but I'll ask for binary then :-) > and for mac addresses, but not for inet4 addresses. I don't know what sort > of address this is. > > 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"