From owner-svn-src-all@FreeBSD.ORG Fri Mar 13 20:06:03 2015 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 648EEA03; Fri, 13 Mar 2015 20:06:03 +0000 (UTC) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 37CCFE76; Fri, 13 Mar 2015 20:06:03 +0000 (UTC) Received: from ralph.baldwin.cx (pool-173-54-116-245.nwrknj.fios.verizon.net [173.54.116.245]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 3CB4DB93B; Fri, 13 Mar 2015 16:06:01 -0400 (EDT) From: John Baldwin To: Ian Lepore Subject: Re: svn commit: r279932 - head/sys/vm Date: Fri, 13 Mar 2015 14:34:30 -0400 Message-ID: <1700119.KBm0D9PSuZ@ralph.baldwin.cx> User-Agent: KMail/4.14.2 (FreeBSD/10.1-STABLE; KDE/4.14.2; amd64; ; ) In-Reply-To: <1426269478.19693.4.camel@freebsd.org> References: <201503121806.t2CI6VSU034853@svn.freebsd.org> <3013452.2FfDYxpIKo@ralph.baldwin.cx> <1426269478.19693.4.camel@freebsd.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Fri, 13 Mar 2015 16:06:01 -0400 (EDT) Cc: "svn-src-head@freebsd.org" , "svn-src-all@freebsd.org" , "src-committers@freebsd.org" , Ryan Stone X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Mar 2015 20:06:03 -0000 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