Date: Fri, 1 Mar 2013 09:13:41 -0800 From: John-Mark Gurney <jmg@funkthat.com> To: freebsd-hackers@freebsd.org Subject: Re: looking for someone to fix humanize_number (test cases included) Message-ID: <20130301171341.GJ55866@funkthat.com> In-Reply-To: <20121225194626.GA57447@volcano.org> References: <mailman.15.1356350401.35494.freebsd-hackers@freebsd.org> <20121225172037.GA56225@volcano.org> <20121225182355.GA56732@volcano.org> <20121225194626.GA57447@volcano.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <jmg@funkthat.com>
> > > > 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."
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130301171341.GJ55866>