From owner-freebsd-hackers@FreeBSD.ORG Fri Mar 25 22:32:03 2011 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 1233) id 35F491065670; Fri, 25 Mar 2011 22:32:03 +0000 (UTC) Date: Fri, 25 Mar 2011 22:32:03 +0000 From: Alexander Best To: Xin LI Message-ID: <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> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Cc: freebsd-hackers@freebsd.org Subject: Re: Switching to [KMGTPE]i prefixes? X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Mar 2011 22:32:03 -0000 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 http://www.delphij.net -- a13x