From owner-svn-src-all@FreeBSD.ORG Fri Feb 13 14:29:53 2015 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 226D16DB; Fri, 13 Feb 2015 14:29:53 +0000 (UTC) Received: from mail106.syd.optusnet.com.au (mail106.syd.optusnet.com.au [211.29.132.42]) by mx1.freebsd.org (Postfix) with ESMTP id D3A5C7ED; Fri, 13 Feb 2015 14:29:52 +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 mail106.syd.optusnet.com.au (Postfix) with ESMTPS id 61F0F3C72E7; Sat, 14 Feb 2015 01:29:50 +1100 (AEDT) Date: Sat, 14 Feb 2015 01:29:50 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Andrey Chernov Subject: Re: svn commit: r278634 - head/lib/libc/gen In-Reply-To: <54DDAEF6.3060900@freebsd.org> Message-ID: <20150214005543.X2210@besplex.bde.org> References: <201502122107.t1CL7gaO004041@svn.freebsd.org> <54DD2A87.2050008@FreeBSD.org> <9A683D99-C1E9-4736-982C-69F583D3A40D@FreeBSD.org> <20150213172738.C1007@besplex.bde.org> <54DDABF2.9000201@freebsd.org> <54DDAEF6.3060900@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=Za4kaKlA c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=uJTURyrQue8Ndf2tMUgA:9 a=CjuIK1q_8ugA:10 Cc: src-committers@freebsd.org, Pedro Giffuni , svn-src-all@freebsd.org, "Bjoern A. Zeeb" , Bruce Evans , svn-src-head@freebsd.org 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: Fri, 13 Feb 2015 14:29:53 -0000 On Fri, 13 Feb 2015, Andrey Chernov wrote: > We even don't need to check arg excepting for < 0, because what is > needed is rlimt_t and not arg. So this version will be better: > > rlimt_t targ; > > if (arg < 0) { > errno = EINVAL; > return (-1); > } This is reasonable, but not encouraged by the API or compatible with what setrlimit() does with negative args. (setrlimit() still uses my hack from 1994, of converting negative args to RLIM_INFINITY. In 4.4BSD, it doesn't even check for negative args, and mostly stores them unchanged; then undefined behaviour tends to occur when the stored values are used without further checking.) In POSIX, rlim_t is an unsigned integer type, so negative args are impossible for it and all negative args here are unrepresentable as an rlim_t. We can convert them to any garbage. The historical garbage is as good as anything. > targ = arg; Unnecessary overflow before checking in the theoretical case where rlim_t is smaller than long. > if (targ > RLIM_INFINITY / 512) > targ = RLIM_INFINITY / 512; When overflow doesn't occur, using targ instead of arg in the comparison does nothing except possibly confuse the compiler into not generating a tautologous-compare warning. Assigning RLIM_INFINITY to an rlim_t instead of to a long is one way to way to fix the overflow case for the latter, > limit.rlim_max = limit.rlim_cur = targ * 512 However, I don't like using rlim_t for the scaled value that is not an rlimit. An incomplete fix with handling of negative values restored is something like: intmax_t targ; targ = arg; if (targ > RLIM_INFINITY / 512) targ = RLIM_INFINITY / 512; limit.rlim_max = limit.rlim_cur = targ * 512 This is still incomplete. The comparison is still obviously tautologous when intmax_t == rlim_t (the amd64 case). If intmax_t is larger than long (the i386 case) or even rlim_t (the notyet case), then it is slightly less obviously tautologous. This can be fixed by sprinkling volatiles, e.g. for targ. Range checking in expr(1) needed much more delicate fixes including sprinkling volatiles. There the problem was that the compiler saw that the behaviour was either undefined or tautologously true or false. Optimization then allows it to remove tautologous checks. The warnings about tautologous checks must have been turned off, or perhaps broken when undefined behaviour is required for the tautologies, else we should have seen the warnings before the optimizations. The warnings are needed even more with the optimizations, to give a hint that necessary code is being removed. Bruce