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 <delp...@delphij.net> http://www.delphij.net
--
a13x
_______________________________________________
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"
