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 <delp...@delphij.net> http://www.delphij.net
>
>
>
> --
> a13x
--
a13x
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 = "";
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
