From owner-svn-src-all@FreeBSD.ORG Thu Dec 19 21:16:07 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 68A0F6EB for ; Thu, 19 Dec 2013 21:16:07 +0000 (UTC) Received: from nm10.bullet.mail.ir2.yahoo.com (nm10.bullet.mail.ir2.yahoo.com [212.82.96.33]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id B73F119FB for ; Thu, 19 Dec 2013 21:16:06 +0000 (UTC) Received: from [212.82.98.53] by nm10.bullet.mail.ir2.yahoo.com with NNFMP; 19 Dec 2013 21:16:01 -0000 Received: from [46.228.39.67] by tm6.bullet.mail.ir2.yahoo.com with NNFMP; 19 Dec 2013 21:16:01 -0000 Received: from [127.0.0.1] by smtp104.mail.ir2.yahoo.com with NNFMP; 19 Dec 2013 21:16:01 -0000 X-Yahoo-Newman-Id: 711372.81079.bm@smtp104.mail.ir2.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: 7aN3Mn4VM1keX5aDXr7VNkRUBScDk8.pf_IIEuMfCl7XShC 4ilJGLJ7fKbB8lntFGHoy5lzAEjlOjxqbeGI.96v4pIN5kJflooZPIiJ34rn S_xgDDNG0GiUzU0H7NO.mLTkQ5w4ZAihlGneb0FmXbXUCi7W9ljxxkyr_j_C FBo2f0c20tkgobljYR1feHUyIZmMngX6aQckGSalQVD7HFbltWqlda7os43y qu._zY9i8RQNYLO.9vcRLODzt7u4SA6rst2Yf8ZihwmN4gabmcpaapJ3tFnL s0Vvpv5LFI_2Kyqp.re6ZDP3D44.4hhh8XbG_D2Za470Ugm.5VPrStFShNlJ ROcnOi_us32YeC2XFA7Afaoi1bq74zAUl9RBSIVUcBnmT9K9PmD1dfL4g43p 37i6dPrBb7qSx2Fuywcw0wYw2fzURqZ8isuSUIPk8_FnN6Lyyz1THl4Edp01 7._ekJdYVQpEWkfUSP9JWZDyHXfueLMMzIZN1sHD_gdqDqkjsXGfnMDUX7NI aHPk_X3.T49eLE1CIDy.MJC0PdOWAeA-- X-Yahoo-SMTP: iDf2N9.swBDAhYEh7VHfpgq0lnq. X-Rocket-Received: from [192.168.119.11] (se@84.154.112.54 with plain [188.125.69.59]) by smtp104.mail.ir2.yahoo.com with SMTP; 19 Dec 2013 21:16:01 +0000 UTC Message-ID: <52B3620A.8050603@freebsd.org> Date: Thu, 19 Dec 2013 22:15:54 +0100 From: Stefan Esser User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Andreas Tobler , Bruce Evans Subject: Re: svn commit: r259609 - head/sys/kern References: <201312190901.rBJ91ko3036881@svn.freebsd.org> <20131219204903.V24189@besplex.bde.org> <52B32647.2030008@freebsd.org> <52B35B37.5070809@FreeBSD.org> In-Reply-To: <52B35B37.5070809@FreeBSD.org> X-Enigmail-Version: 1.6 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.17 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: Thu, 19 Dec 2013 21:16:07 -0000 Am 19.12.2013 21:46, schrieb Andreas Tobler: > On 19.12.13 18:00, Stefan Esser wrote: > >> I'd replace the two occurances of LLONG_MAX with INT64_MAX and add the >> missing empty line: >> >> static __inline sbintime_t >> timer2sbintime(intptr_t data) >> { >> >> if (data > INT64_MAX / SBT_1MS) >> return INT64_MAX; >> return (SBT_1MS * data); >> } >> >> If you can show evidence that a limit of INT64_MAX/2 is more appropriate >> (2^30 seconds or 34 years), the limit could be of course be reduced to >> that value. >> >> I could not find any code that would not tolerate INT64_MAX, though ... > > Aehm, what about 32-bit systems where intptr_t == __int32_t? > > cc1: warnings being treated as errors > /export/devel/fbsd/src/sys/kern/kern_event.c: In function 'timer2sbintime': > /export/devel/fbsd/src/sys/kern/kern_event.c:529: warning: comparison is > always false due to limited range of data type You are right, this needs to be fixed, too :( I see two possibilities: 1) Conditional compilation: There is no need to check an upper bound on ILP32 architectures. INT32_MAX seconds can be expressed as sbintime. Architectures where intptr_t has at least 48 bits will support the maximum time that can be expressed in sbintime and the comparison can be left as is for them. 2) Calculate the upper bound in such a way, that it is guaranteed to be lower than the highest value that can be expressed as intptr_t. This value is (INT32_MAX - 1) on 32 bit architectures, while it remains (INT64_MAX / SBT_1MS) on 64 bit architectures. I'm afraid that the warning emitted for 32 bit architectures will cause tinderbox build failures, but I haven't seen a failure message, yet. Should I back-out the commit that added the range check? As long as -fno-strict-overflow is not put back into CFLAGS, the test is not strictly required (only for the extremely unlikely case that a number > 1000 * MAX32_INT results in an extremely short remainder after multiplication with SBT_1MS modulo 32 ...). Regards, STefan NB: I should have known better and should have asked for a review of this change before it wa committed. Sorry for the inconvenience caused :(