From owner-svn-src-all@FreeBSD.ORG Tue Feb 3 16:52:05 2015 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 8C1A7B5E; Tue, 3 Feb 2015 16:52:05 +0000 (UTC) Received: from mail107.syd.optusnet.com.au (mail107.syd.optusnet.com.au [211.29.132.53]) by mx1.freebsd.org (Postfix) with ESMTP id 1E88A97C; Tue, 3 Feb 2015 16:52:04 +0000 (UTC) Received: from c211-30-166-197.carlnfd1.nsw.optusnet.com.au (c211-30-166-197.carlnfd1.nsw.optusnet.com.au [211.30.166.197]) by mail107.syd.optusnet.com.au (Postfix) with ESMTPS id 66F2CD4308F; Wed, 4 Feb 2015 03:51:56 +1100 (AEDT) Date: Wed, 4 Feb 2015 03:51:55 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Pedro Giffuni Subject: Re: svn commit: r278154 - head/lib/msun/src In-Reply-To: <54D0F29C.2090401@FreeBSD.org> Message-ID: <20150204031811.N2584@besplex.bde.org> References: <201502031417.t13EHU9g074687@svn.freebsd.org> <20150204012614.T1972@besplex.bde.org> <54D0F29C.2090401@FreeBSD.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=baJSDo/B c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=EA4qJ_KTQ518DKzBkYcA:9 a=CjuIK1q_8ugA:10 Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Bruce Evans X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18-1 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: Tue, 03 Feb 2015 16:52:05 -0000 On Tue, 3 Feb 2015, Pedro Giffuni wrote: > On 03/02/2015 10:38 a.m., Bruce Evans wrote: Please trim quotes. [... quotes truncated] >> You mean, take a step backwards to something that is harder to understand. >>> Modified: head/lib/msun/src/s_scalbln.c >>> ============================================================================== >>> --- head/lib/msun/src/s_scalbln.c Tue Feb 3 13:45:06 2015 (r278153) >>> +++ head/lib/msun/src/s_scalbln.c Tue Feb 3 14:17:29 2015 (r278154) >>> @@ -35,7 +35,9 @@ scalbln (double x, long n) >>> { >>> int in; >>> >>> - in = (n > INT_MAX) ? INT_MAX : (n < INT_MIN) ? INT_MIN : n; >> >> It is a feature that this detected a bug in Coverity. I intentionally >> left out the cast of n at the end, after even more intentionally leaving >> it out at the beginning. It is obvious to humans^WC programmers, that >> n has been checked to fit in an int; thus any compiler warning about n >> possibly being truncated is a compiler defect. It is especially important >> that compilers understand explicit range checks and don't warn about >> conversions that cannot change the value when the range checks have been >> satisfied. > > I noticed the implicit cast, but then the fact that you triggered a bug in > coverity, > could be an indication that you could also trigger the same bug in a compiler > optimizer so some bogus compiler may end up optimizing the above check into > /*do nothing */. Not likely. >>> + in = (int)n; >> >> This restores a hard-to understand cast. Compilers cannot reasonably >> understand this at all. Without the cast, they should (optionally) >> warn about possible truncation. But the truncation is intentional. >> The programmer is using the cast to inhibit the warning (the case has no >> other effect). A higher level of warnings would still warn about it, >> and my rewrite was to prevent this and to make the code easier to >> understand. >> ... > I honestly only wanted to clean the original bug but all in all, I find our > implementation of this functions somewhat disappointing: shouldn't we > be doing real calculations with the requested precision instead of wrapping > them around the int counterparts? It looks like NetBSD does implement them. That would be a silly micro-optimization for functions that are almost never used. We are doing real calculations, with a simple translation for overflowing and undeflowing cases to avoid duplicating code. Now I wonder why these functions (the ones taking a long arg) even exist. According to a Linux man page which seems to be quoting a POSIX spec, it is to support systems with 16-bit ints and unusually large LDBL_MAX_EXP. FreeBSD only supports systems where LDBL_MAX_EXP is at most 16384, and this fits in a 16-bit int. Very little code supports these unusual systems. Most code uses plain assumes 16-bit ints and/or usual LDBL_MAX_EXP, and just uses scalbn*(). POSIX now requires 32-bit ints, so this rationale certainly doesn't apply to it. It applies to C99. So the sloppy code is now portable in POSIX but not in C99. Bruce