svn commit: r279932 - head/sys/vm

[John Baldwin]

On Friday, March 13, 2015 11:57:58 AM Ian Lepore wrote:
> On Fri, 2015-03-13 at 13:19 -0400, John Baldwin wrote:
> > On Friday, March 13, 2015 10:14:27 AM Ian Lepore wrote:
> > > On Fri, 2015-03-13 at 06:24 -0400, John Baldwin wrote:
> > > > On Thursday, March 12, 2015 05:24:51 PM Ian Lepore wrote:
> [...]
> > > 
> > > In general I'm glad I got called away to an onsite meeting yesterday and
> > > didn't get far with these changes, because the more I think about it,
> > > the less satisfied I am with this expedient fix.  The other fix I
> > > started on, where a new SBUF_COUNTNUL flag can be set to inform the
> > > sbuf_finish() code that you want the terminating nul counted in the data
> > > length just feels like a better fit for the overall "automaticness" of
> > > how the sbuf stuff works.
> > 
> > Hmm, I actually think that it's a bug that the terminating nul isn't included
> > when draining.  If we fixed that then I think that fixes most of these?
> > The places that explicitly use 'sysctl_handle_string()' with an sbuf
> > should probably just be using sbuf_len(sb) + 1' explicitly.  (Another
> > option would be to have a sysctl_handle_sbuf() that was a wrapper around
> > sysctl_handle_string() that included the + 1 to hide that detail if there is
> > more than one.)
> > 
> 
> Some of the uses of sbuf for sysctl use sbuf_bcat() for dealing with
> binary structs, so we can't just assume that a nullterm should be added
> and included in the buffer length -- there needs to be some mechanism to
> say explicitly "this is an sbuf for a sysctl string" (and more generally
> "this is an sbuf for a string where I want the nul byte counted as part
> of the data" because that could be useful in non-sysctl contexts too,
> especially in userland).

Humm, that would seem to be an abuse of the API really.  It is specifically
designed for strings as someone else noted at the start of this thread (and
as noted in the manpage).  If anything I'd argue that the use cases that don't
want a string should be the ones that should get a special flag in that case
(or perhaps we should have a different little API to manage a buffer used for
a draining sysctl where the data is a binary blob instead of a string).  If
you agree I'm happy to do some of the work (e.g. the different wrapper API).

-- 
John Baldwin