From owner-svn-src-head@FreeBSD.ORG Fri Feb 13 16:47:04 2015 Return-Path: Delivered-To: svn-src-head@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 D4271C3; Fri, 13 Feb 2015 16:47:03 +0000 (UTC) Received: from mail109.syd.optusnet.com.au (mail109.syd.optusnet.com.au [211.29.132.80]) by mx1.freebsd.org (Postfix) with ESMTP id 8B16A91B; Fri, 13 Feb 2015 16:47:03 +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 mail109.syd.optusnet.com.au (Postfix) with ESMTPS id A2C59D635B3; Sat, 14 Feb 2015 03:47:00 +1100 (AEDT) Date: Sat, 14 Feb 2015 03:46:53 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Pedro Giffuni Subject: Re: svn commit: r278634 - head/lib/libc/gen In-Reply-To: <54DE1FC9.4000503@FreeBSD.org> Message-ID: <20150214032839.E3221@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> <20150214005543.X2210@besplex.bde.org> <54DE1FC9.4000503@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=CDFkk6uA_NIjEHA-TEwA:9 a=CjuIK1q_8ugA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Andrey Chernov , Bruce Evans X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Feb 2015 16:47:04 -0000 On Fri, 13 Feb 2015, Pedro Giffuni wrote: > On 02/13/15 09:29, Bruce Evans wrote: >> 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.) > > Actually I think the above check would be OK according to POSIX: > ... > > The /ulimit/() function shall fail and the limit shall be unchanged if: > > [EINVAL] > The /cmd/ argument is not valid. > ... I already partly explained that this is (part of) why POSIX discourages returning EINVAL for the /data/ argument. EINVAL is for the /cmd/ argument. No errno is specified for the /data/ argument. Instead, the implementation is implicitly encourage to (if the requested value is unrepresentable) invent some representable value and return the result of setting it. We still often get EPERM if our invented value cannot be set due to EPERM. Rounding makes EPERM even more likely than ususal. E.g., if we start with RLIM_INFINITY and get and set it using some implementations of this function, then rounding reduces the hard rlimit. Then if a slightly different implementation tries to increase the hard rlimit hack to RLIM_INFINITY, then this fails with EPERM (except for root). Some preliminary attempts to fix the warning would have caused this EPERM error for almost all error cases, since non-error cases rounded down but error cases attempted to raise to RLIM_INFINITY. > ... >> 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. > > I am passing this (with the check for negative values and __intmax_t) > through the tinderbox. > FWIW, I had something else that managed to compile but is *very* > ugly and can cause an effect similar to tear gas on sensitive eyes ;). I also forgot to include for the declaration of intmax_t. Use of double underscores in applications is also bad for the eyes. Bruce