Date: Fri, 25 Mar 2011 22:40:27 +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: <20110325224027.GA859@freebsd.org> In-Reply-To: <20110325223203.GA95976@freebsd.org> 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> <20110325223203.GA95976@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--C7zPtVaVf+AK4Oqc Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri Mar 25 11, Alexander Best wrote: > On Fri Mar 25 11, Xin LI wrote: > > FYI I have a patch and I have incorporated some of Alexander's idea. this is what i had in mind (see attached patch). cheers. alex > > > > 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 -- a13x --C7zPtVaVf+AK4Oqc Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="humanize_number.c.diff" diff --git a/lib/libutil/humanize_number.c b/lib/libutil/humanize_number.c index 75bcb46..e086478 100644 --- a/lib/libutil/humanize_number.c +++ b/lib/libutil/humanize_number.c @@ -33,11 +33,8 @@ #include <sys/cdefs.h> __FBSDID("$FreeBSD$"); -#include <sys/types.h> -#include <assert.h> #include <inttypes.h> #include <stdio.h> -#include <stdlib.h> #include <string.h> #include <locale.h> #include <libutil.h> @@ -51,50 +48,50 @@ humanize_number(char *buf, size_t len, int64_t quotient, int64_t divisor, max; size_t baselen; - assert(buf != NULL); - assert(suffix != NULL); - assert(scale >= 0); - remainder = 0; - if (flags & HN_DIVISOR_1000) { - /* SI for decimal multiplies */ - divisor = 1000; + if (flags & HN_IEC_PREFIXES) { + baselen = 2; + divisor = 1024; if (flags & HN_B) - prefixes = "B\0k\0M\0G\0T\0P\0E"; + prefixes = "B\0\0Ki\0Mi\0Gi\0Ti\0Pi\0Ei"; else - prefixes = "\0\0k\0M\0G\0T\0P\0E"; - } else { - /* - * binary multiplies - * XXX IEC 60027-2 recommends Ki, Mi, Gi... - */ - divisor = 1024; + prefixes = "\0\0Ki\0Mi\0Gi\0Ti\0Pi\0Ei"; + else { + baselen = 1; + if (flags & HN_DIVISOR_1000) + divisor = 1000; + else + /* default case */ + divisor = 1024; if (flags & HN_B) - prefixes = "B\0K\0M\0G\0T\0P\0E"; + prefixes = "B\0\0k\0\0M\0\0G\0\0T\0\0P\0\0E"; else - prefixes = "\0\0K\0M\0G\0T\0P\0E"; + prefixes = "\0\0\0k\0\0M\0\0G\0\0T\0\0P\0\0E"; } -#define SCALE2PREFIX(scale) (&prefixes[(scale) << 1]) - maxscale = 7; +#define SCALE2PREFIX(scale) (&prefixes[(scale) * 3]) + maxscale = 7; /* XXX cannot scale past `exa' */ - if (scale >= maxscale && - (scale & (HN_AUTOSCALE | HN_GETSCALE)) == 0) + 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)) + return (-1); + if (len > 0) buf[0] = '\0'; if (quotient < 0) { sign = -1; quotient = -quotient; - baselen = 3; /* sign, digit, prefix */ + baselen += 2; /* sign, digit */ } else { sign = 1; - baselen = 2; /* digit, prefix */ + baselen += 1; /* digit */ } if (flags & HN_NOSPACE) sep = ""; --C7zPtVaVf+AK4Oqc--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110325224027.GA859>