From owner-freebsd-hackers@FreeBSD.ORG Thu Dec 27 18:31:40 2012 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 28066917 for ; Thu, 27 Dec 2012 18:31:40 +0000 (UTC) (envelope-from cliftonr@volcano.org) Received: from hrndva-omtalb.mail.rr.com (hrndva-omtalb.mail.rr.com [71.74.56.122]) by mx1.freebsd.org (Postfix) with ESMTP id CBA678FC0A for ; Thu, 27 Dec 2012 18:31:39 +0000 (UTC) X-Authority-Analysis: v=2.0 cv=Jaw+XD2V c=1 sm=0 a=nLSj6wnnDCA9frvL/Tt8NQ==:17 a=3pwD0LKv0YcA:10 a=z1TLwsU0kBEA:10 a=WthmilfBrBsA:10 a=sLPLfFCmqb0A:10 a=kj9zAlcOel0A:10 a=GjEiR67sAAAA:8 a=vbqy14apGeAA:10 a=6I5d2MoRAAAA:8 a=uyavkMrdAAAA:8 a=4FFUHJPBAAAA:8 a=pGLkceISAAAA:8 a=jb__rZ8GAAAA:8 a=xERaT4hCXWtQ0p4kfbsA:9 a=CjuIK1q_8ugA:10 a=lQ7BwGF-Y-wA:10 a=SV7veod9ZcQA:10 a=JGX6LFFZUg8A:10 a=WglDhLyllLQA:10 a=Ke08FT2oSu0A:10 a=sHp_62vNEjwA:10 a=YWxM20KZKtQXhz3x:21 a=E3B3KeU8iniZbwx2:21 a=nLSj6wnnDCA9frvL/Tt8NQ==:117 X-Cloudmark-Score: 0 X-Authenticated-User: X-Originating-IP: 70.95.171.134 Received: from [70.95.171.134] ([70.95.171.134:57720] helo=oz.volcano.org) by hrndva-oedge01.mail.rr.com (envelope-from ) (ecelerity 2.2.3.46 r()) with ESMTP id F5/35-15459-A049CD05; Thu, 27 Dec 2012 18:31:38 +0000 Received: by oz.volcano.org (Postfix, from userid 1001) id DC7235082E; Thu, 27 Dec 2012 08:31:36 -1000 (HST) Date: Thu, 27 Dec 2012 08:31:36 -1000 From: Clifton Royston To: Eitan Adler , freebsd-hackers@freebsd.org, John-Mark Gurney Subject: Re: looking for someone to fix humanize_number (test cases included) Message-ID: <20121227183136.GA77509@volcano.org> Mail-Followup-To: Eitan Adler , freebsd-hackers@freebsd.org, John-Mark Gurney References: <20121226205307.GA66528@volcano.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20121226205307.GA66528@volcano.org> User-Agent: Mutt/1.4.2.3i X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Dec 2012 18:31:40 -0000 On Wed, Dec 26, 2012 at 10:53:07AM -1000, Clifton Royston wrote: > On Wed, Dec 26, 2012 at 12:00:01PM +0000, freebsd-hackers-request@freebsd.org wrote: > > Date: Tue, 25 Dec 2012 14:52:09 -0500 > > From: Eitan Adler > > To: freebsd-hackers@freebsd.org, John-Mark Gurney > > Subject: Re: looking for someone to fix humanize_number (test cases included) > > Message-ID: > > > > Content-Type: text/plain; charset=UTF-8 > > > > On 25 December 2012 14:46, Clifton Royston wrote: > > >> I correct myself: the function works fine, and there are no bugs I > > > could find, though it's clear the man page could emphasize the correct > > > usage a bit more. > > > > Can you submit a diff to the man page as well? I figure if you got > > confused at least 10 others got even more confused. > > I'd be happy to, and will do so soon. ... Update - I've much expanded the test cases, up to around 120 so far, although they don't yet cover HN_GETSCALE, HN_DECIMAL, HN_NOSPACE, or HN_B. I may put in some rudimentary tests for HN_GETSCALE but don't plan to cover the latter three at all, although HN_NOSPACE and HN_B are pretty trivial. I've also added checking of expected vs. actual return values. I'll put the updated test program somewhere shortly. In the current library routine, these tests have found one more piece of arguable misdesign for discussion, and several out-and-out bugs. 1) Possible misdesign - the current implementation raises an assertion and coredumps the calling program on a negative scale value; other invalid scale values simply return -1. I'd argue both should do the same thing, and I think the latter is less astonishing. (It's not like passing a null buffer pointer IMHO.) It's not a huge issue though so I'd be willing to leave it. The new test program has an option to enable testing of negative scale values. 2) Bug: If compiled with NDEBUG to disable the assert, negative values of scale < 0 would not be checked for. I am fixing this. 3) Bug: the only valid scale values are 0-7, HN_GETSCALE, and HN_AUTOSCALE; no combinations of these are valid. (0 does no scaling and acts as a glorified printf, so is rarely useful.) However, any number which happens to have the HN_GETSCALE and/or HN_AUTOSCALE bit set will be accepted and likely do something astonishing for the caller. I am fixing this so only the specified values are accepted. 4) Documentation: needs to specify what I just wrote about scale values, plus what must be true to avoid assertion failures, as well as points raised above about values passed via scale vs. flags parms. 5) Bug: number values in the range of .5 exa[byte] and above are never or rarely handled correctly; the code does some multiplies up front which will cause them to wrap and return completely bogus values. I haven't tried to fix this as yet. The new test program has a -E option to enable testing of exabyte range number values. Obviously nobody's using our utils on exabyte-sized filesystems as yet... The new test program has a -E option to enable testing of exabyte range number values. 6) Undefined case: when the function exits due to an invalid scale value, it does not set the buffer (if valid) to an empty string. I intend to have it do that, mostly to make it more convenient for testing. -- Clifton Comparison of results from libutil vs. my current version: [cliftonr@oz ~/humanize_number]$ ./test_humanize_numbers -E wrong return value on index 6, got: 19 + "-53", expected 3 + "1 E"; num = 500000000000000000, scale = 32, flags= HN_DIVISOR_1000. result mismatch on index 13, got: "0 E", expected "2 E"; num = 1500000000000000000, scale = 32, flags= HN_DIVISOR_1000. result mismatch on index 20, got: "0 E", expected "1 E"; num = 576460752303423488, scale = 32, flags= . result mismatch on index 27, got: "0 E", expected "2 E"; num = 1729382256910270464, scale = 32, flags= . result mismatch on index 48, got: "0 E", expected "1 E"; num = 500000000000000000, scale = 6, flags= HN_DIVISOR_1000. result mismatch on index 60, got: "0 E", expected "2 E"; num = 1500000000000000000, scale = 6, flags= HN_DIVISOR_1000. result mismatch on index 72, got: "0 E", expected "1 E"; num = 576460752303423488, scale = 6, flags= . result mismatch on index 84, got: "0 E", expected "2 E"; num = 1729382256910270464, scale = 6, flags= . result mismatch on index 85, got: "0 E", expected ""; num = 1, scale = 7, flags= . result mismatch on index 86, got: "0 E", expected ""; num = 1, scale = 7, flags= HN_DIVISOR_1000. expected error on index 87, but got: 2 + "1 "; num = 1, scale = 1000, flags= . expected error on index 88, but got: 2 + "1 "; num = 1, scale = 1000, flags= HN_DIVISOR_1000. result mismatch on index 89, got: "1 ", expected ""; num = 0, scale = 1000000, flags= . result mismatch on index 90, got: "1 ", expected ""; num = 0, scale = 1000000, flags= HN_DIVISOR_1000. expected error on index 91, but got: 0 + ""; num = 0, scale = 2147483647, flags= . expected error on index 92, but got: 0 + ""; num = 0, scale = 2147483647, flags= HN_DIVISOR_1000. total errors: 16/117 tests [cliftonr@oz ~/humanize_number]$ ./test_humanize_numbers_new -E -n wrong return value on index 6, got: 19 + "-53", expected 3 + "1 E"; num = 500000000000000000, scale = 32, flags= HN_DIVISOR_1000. result mismatch on index 13, got: "0 E", expected "2 E"; num = 1500000000000000000, scale = 32, flags= HN_DIVISOR_1000. result mismatch on index 20, got: "0 E", expected "1 E"; num = 576460752303423488, scale = 32, flags= . result mismatch on index 27, got: "0 E", expected "2 E"; num = 1729382256910270464, scale = 32, flags= . result mismatch on index 48, got: "0 E", expected "1 E"; num = 500000000000000000, scale = 6, flags= HN_DIVISOR_1000. result mismatch on index 60, got: "0 E", expected "2 E"; num = 1500000000000000000, scale = 6, flags= HN_DIVISOR_1000. result mismatch on index 72, got: "0 E", expected "1 E"; num = 576460752303423488, scale = 6, flags= . result mismatch on index 84, got: "0 E", expected "2 E"; num = 1729382256910270464, scale = 6, flags= . total errors: 8/123 tests -- Clifton Royston -- cliftonr@iandicomputing.com / cliftonr@volcano.org President - I and I Computing * http://www.iandicomputing.com/ Custom programming, network design, systems and network consulting services