Date: Fri, 25 Mar 2011 22:32:03 +0000 From: Alexander Best <arundel@freebsd.org> To: Xin LI <delphij@gmail.com> Cc: freebsd-hackers@freebsd.org Subject: Re: Switching to [KMGTPE]i prefixes? Message-ID: <20110325223203.GA95976@freebsd.org> In-Reply-To: <AANLkTinEcT__Wtc6LkSyqqMnQwuKVUbZC4dPZvZH_dSX@mail.gmail.com> References: <20110325002115.GA323@freebsd.org> <20110325015508.GA14565@freebsd.org> <20110325024658.GA19544@freebsd.org> <336A9ACD-29BF-41C9-BC25-917CC1E4587D@bsdimp.com> <20110325195325.GA69264@freebsd.org> <AANLkTinEcT__Wtc6LkSyqqMnQwuKVUbZC4dPZvZH_dSX@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri Mar 25 11, Xin LI wrote:
> FYI I have a patch and I have incorporated some of Alexander's idea.
>
> Difference:
>
> - Use of both HN_DIVISOR_1000 and HN_IEC_PREFIXES triggers an
> assertion. I think it doesn't make sense to return since this is an
> API violation and we should just tell the caller explicitly;
actually i vote for removing all asserts in humanize_number.c and return -1
based upon the later checks.
the existing
assert(buf != NULL);
assert(suffix != NULL);
checks aren't really needed, since buf and suffix are also checked later on. so
just having:
if (scale <= 0 || (scale >= maxscale &&
(scale & (HN_AUTOSCALE | HN_GETSCALE)) == 0))
return (-1);
if (buf == NULL || suffix == NULL)
return (-1);
if ((flags & (HN_DIVISOR_1000 | HN_IEC_PREFIXES)) == 0)
return (-1);
...should be enough.
> - DIVISOR_1000 and !1000 cases use just same prefixes, so merge them
> while keeping divisor intact;
good idea. however i think you should add a comment to point out that the
default behavior is !DIVISOR_1000 && !HN_IEC_PREFIXES. one has to look very
closely to find out.
> - Make prefixes table consistently long. I have no strong opinion on
> this one, though, it's just what my original version used and I can
> change it to the way Alexander did if there is an advantage of doing
> that way.
i think the way you're doing it is nicer than how i implemented it.
>
> (Note, it seems that we use HN_ prefix for both 'scale' and 'flags', I
> have sorted them by value but HN_IEC_PREFIXES should really belong to
> the flags group).
this is odd indeed. actually the possible 'scale' and 'flags' flags should
not have the same prefixes. but it appears we're stuck with this.
i think sorting me should sort them into the two groups and not value wise.
so it's
#define HN_DECIMAL 0x01
#define HN_NOSPACE 0x02
#define HN_B 0x04
#define HN_DIVISOR_1000 0x08
#define HN_IEC_PREFIXES 0x40
#define HN_GETSCALE 0x10
#define HN_AUTOSCALE 0x20
>
> Cheers,
> --
> Xin LI <delphij@delphij.net> http://www.delphij.net
--
a13x
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110325223203.GA95976>