[ccan] [PATCH 1/3] opt: add integer helpers that accept k, M, G, T, P, E suffixes

[Rusty Russell]

On Wed, 03 Aug 2011 23:43:29 +1200, Douglas Bagnall <douglas at paradise.net.nz> wrote:
> 
> These functions come in two flavours: those ending with "_si", which
> have 1000-based interpretations of the suffixes; and those ending with
> "_bi", which use base 1024.  There are versions for signed and
> unsigned int, long, and long long destinations, with tests for all 12
> new functions.  The tests get a bit repetitive, I am afraid.

That's OK, I'm just happy they exist!

> The arithmetic for unsigned variations is actually done using signed
> long long integers, so the maximum possible value is LLONG_MAX, not
> ULLONG_MAX.  This follows the practice of existing functions, and 
> avoids tedious work.

Excellent.  I've applied all three, sorry for the delay.

One nasty bug showed up in the final patch when testing on 32 bit:

not ok 204 - strcmp(buf, "5P") == 0
#     Failed test (/home/rusty/devel/cvs/ccan/ccan/opt/test/run-helpers.c:main() at line 505)

The answer given was "5120T" instead of "5P".  I couldn't see it either,
but walking through with the debugger showed it leaving this loop early:

	const char *suffixes = "kMGTPE";
	int i;
	if (ll == 0){
		/*zero is special because everything divides it (you'd get "0E")*/
		snprintf(buf, OPT_SHOW_LEN, "0");
		return;
	}
	for (i = 0; i < sizeof(suffixes); i++){
		long long tmp = ll / base;
		if (tmp * base != ll)
			break;
		ll = tmp;
	}

I changed sizeof(suffixes) to strlen(suffixes) (rather than changing
suffixes to an array), which is one less than you meant, but more
correct.

Gcc will optimize it anyway (alternately, ARRAY_SIZE(suffixes) from
ccan/array_size would have caught this).

Thanks!
Rusty.