From owner-freebsd-hackers@FreeBSD.ORG Fri Mar 1 17:13:42 2013 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id B8765568 for ; Fri, 1 Mar 2013 17:13:42 +0000 (UTC) (envelope-from jmg@h2.funkthat.com) Received: from h2.funkthat.com (gate2.funkthat.com [208.87.223.18]) by mx1.freebsd.org (Postfix) with ESMTP id 8BA24A72 for ; Fri, 1 Mar 2013 17:13:42 +0000 (UTC) Received: from h2.funkthat.com (localhost [127.0.0.1]) by h2.funkthat.com (8.14.3/8.14.3) with ESMTP id r21HDfHt043068 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 1 Mar 2013 09:13:41 -0800 (PST) (envelope-from jmg@h2.funkthat.com) Received: (from jmg@localhost) by h2.funkthat.com (8.14.3/8.14.3/Submit) id r21HDfIs043067 for freebsd-hackers@freebsd.org; Fri, 1 Mar 2013 09:13:41 -0800 (PST) (envelope-from jmg) Date: Fri, 1 Mar 2013 09:13:41 -0800 From: John-Mark Gurney To: freebsd-hackers@freebsd.org Subject: Re: looking for someone to fix humanize_number (test cases included) Message-ID: <20130301171341.GJ55866@funkthat.com> Mail-Followup-To: freebsd-hackers@freebsd.org References: <20121225172037.GA56225@volcano.org> <20121225182355.GA56732@volcano.org> <20121225194626.GA57447@volcano.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20121225194626.GA57447@volcano.org> User-Agent: Mutt/1.4.2.3i X-Operating-System: FreeBSD 7.2-RELEASE i386 X-PGP-Fingerprint: 54BA 873B 6515 3F10 9E88 9322 9CB1 8F74 6D3F A396 X-Files: The truth is out there X-URL: http://resnet.uoregon.edu/~gurney_j/ X-Resume: http://resnet.uoregon.edu/~gurney_j/resume.html X-to-the-FBI-CIA-and-NSA: HI! HOW YA DOIN? can i haz chizburger? X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.2.2 (h2.funkthat.com [127.0.0.1]); Fri, 01 Mar 2013 09:13:41 -0800 (PST) X-Mailman-Approved-At: Fri, 01 Mar 2013 17:34:19 +0000 X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Mar 2013 17:13:42 -0000 Clifton Royston wrote this message on Tue, Dec 25, 2012 at 09:46 -1000: > On Tue, Dec 25, 2012 at 08:23:55AM -1000, Clifton Royston wrote: > > On Tue, Dec 25, 2012 at 07:20:37AM -1000, Clifton Royston wrote: > > > On Mon, Dec 24, 2012 at 12:00:01PM +0000, freebsd-hackers-request@freebsd.org wrote: > > > > From: John-Mark Gurney > > > > To: hackers@FreeBSD.org > > > > Subject: looking for someone to fix humanize_number (test cases > > > > included) > > > > > > > > I'm looking for a person who is interested in fixing up humanize_number. > > ... > > > > So I decided to write a test program to test the output, and now I'm even > > > > more surprised by the output... Neither 7.2-R nor 10-current give what > > > > I expect are the correct results... > > ... > > > > I am bemused. > > I correct myself: the function works fine, and there are no bugs I > could find, though it's clear the man page could emphasize the correct > usage a bit more. > > I had to read the source several times and start on debugging it > before I understood the correct usage of the flag values with the scale > and flags parameters, despite the man page stating: > > The following flags may be passed in scale: > > HN_AUTOSCALE Format the buffer using the lowest multiplier pos- > sible. > HN_GETSCALE Return the prefix index number (the number of > times number must be divided to fit) instead of > formatting it to the buffer. > > The following flags may be passed in flags: > > HN_DECIMAL If the final result is less than 10, display it > using one digit. > ... > HN_DIVISOR_1000 Divide number with 1000 instead of 1024. > > That is, certain flags must be passed in flags and others must only > be passed in scale - a bit counter-intuitive. Also, scale == 0 is > clearly not interpreted as AUTOSCALE, but I am not yet clear how it is > being handled - it seems somewhat like AUTOSCALE but not identical. > > When the test program constant table is updated to pass the scale > flags as specified, as well as fixing the bugs mentioned in the > previous emails, it all passes except for the one (intentional?) > inconsistency that "k" is used in place of "K" if HN_DECIMAL is > enabled. > > The bug in the transfer speed results which prompted this inquiry > suggests that perhaps some clients of humanize_number in the codebase > are also passing the scale parameters incorrectly. I would propose > accepting HN_AUTOSCALE and HN_GETSCALE in the flags field (they don't > overlap with other values) while continuing to accept them in the scale > field for backwards compatibility. Trivial diff below. Sorry I didn't get back to this, but now I have a few minutes... > + getscale = (flags | scale) & HN_GETSCALE; This isn't good: #define HN_IEC_PREFIXES 0x10 #define HN_GETSCALE 0x10 If someone sets HN_IEC_PREFIXES, they'll acidentally enable _GETSCALE.. We could do something anoying by changing the value of _GETSCALE, and then leaving some legacy code to accept the old _GETSCALE on the scale input... This would let new code work, but would break new code on old libraries... So, I don't see an easy way to fix this... -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not."