From owner-svn-src-all@FreeBSD.ORG Sun Aug 15 16:02:27 2010 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8507C1065670; Sun, 15 Aug 2010 16:02:27 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail01.syd.optusnet.com.au (mail01.syd.optusnet.com.au [211.29.132.182]) by mx1.freebsd.org (Postfix) with ESMTP id 1EAA28FC15; Sun, 15 Aug 2010 16:02:26 +0000 (UTC) Received: from c122-106-147-41.carlnfd1.nsw.optusnet.com.au (c122-106-147-41.carlnfd1.nsw.optusnet.com.au [122.106.147.41]) by mail01.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id o7FG2N2w013608 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 16 Aug 2010 02:02:24 +1000 Date: Mon, 16 Aug 2010 02:02:23 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Dag-Erling Smorgrav In-Reply-To: <201008151455.o7FEtW75063515@svn.freebsd.org> Message-ID: <20100816010236.U14975@delplex.bde.org> References: <201008151455.o7FEtW75063515@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r211338 - head/lib/libutil X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 15 Aug 2010 16:02:27 -0000 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