Date: Mon, 16 Aug 2010 02:02:23 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Dag-Erling Smorgrav <des@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r211338 - head/lib/libutil Message-ID: <20100816010236.U14975@delplex.bde.org> In-Reply-To: <201008151455.o7FEtW75063515@svn.freebsd.org> References: <201008151455.o7FEtW75063515@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 15 Aug 2010, Dag-Erling Smorgrav wrote:
> Log:
> no-op commit to note that the example given in the previous commit is
> a very bad one, since the shift does not actually overflow.
Indeed. You barely escaped more mail about it :-).
> This is
> a better example (assuming uint64_t = unsigned long long):
>
> ~0LLU >> 9 = 0x7fffffffffffffLLU
> ~0LLU >> 9 << 10 = 0xfffffffffffffc00LLU
> ~0LLU >> 9 << 10 >> 10 = 0x3fffffffffffffLLU
>
> Modified:
> head/lib/libutil/expand_number.c
>
> Modified: head/lib/libutil/expand_number.c
> ==============================================================================
> --- head/lib/libutil/expand_number.c Sun Aug 15 14:50:03 2010 (r211337)
> +++ head/lib/libutil/expand_number.c Sun Aug 15 14:55:32 2010 (r211338)
> @@ -66,7 +66,7 @@ expand_number(const char *buf, uint64_t
> return (0);
> }
>
> -#define SHIFT(n, b) \
> +#define SHIFT(n, b) \
> do { if (((n << b) >> b) != n) goto overflow; n <<= b; } while (0)
>
> switch (tolower((unsigned char)*endptr)) {
>
Strangely enough, this is now equivalent to the range check in dd
(expand_number() copied many small bugs from dd/args.c and added many
large ones including breaking both range checks).
dd supports a more general multiplier and uses multiply/divide instead
of left-shift/right-shift in the check. For the more general multiplier,
it is more clear that the simple check which was used, i.e., ((number
<< 10) < number), doesn't work.
I wrote or fixed the overflow checks in dd (except for the one that
allows undefined behaviour), but for the multiplier one I now prefer
direct range checking instead of possibly letting overflow occur and
then checking that it didn't occur. Overflow gives undefined behaviour
for negative numbers, so there is no alternative for them. This turns
out to be simplest anyway. Overflow will occur iff n exceeds some
threshold, and it is easy to calculate the threshold. In the above,
the threshold is something like (UINTMAX_MAX >> 10), and in dd it is
something like (UINTMAX_MAX / mult) for the unsigned case, or 2
thresholds (INTMAX_MIN / mult) and (INTMAX_MAX / mult) for the signed
case. This method is already used in FreeBSD's implementation of the
strtol() family.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100816010236.U14975>