Date: Sat, 14 Feb 2015 03:46:53 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Pedro Giffuni <pfg@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Andrey Chernov <ache@freebsd.org>, Bruce Evans <brde@optusnet.com.au> Subject: Re: svn commit: r278634 - head/lib/libc/gen Message-ID: <20150214032839.E3221@besplex.bde.org> In-Reply-To: <54DE1FC9.4000503@FreeBSD.org> References: <201502122107.t1CL7gaO004041@svn.freebsd.org> <BF5F2941-52F5-41A4-8723-E316919718EE@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>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <stdint.h> for the declaration of intmax_t.
Use of double underscores in applications is also bad for the eyes.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150214032839.E3221>