Switching to [KMGTPE]i prefixes?

[Alexander Best]

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 at delphij.net> http://www.delphij.net
> 
> 
> 
> -- 
> a13x

-- 
a13x
-------------- next part --------------
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 = "";