Date: Fri, 13 Feb 2015 12:01:23 -0500 From: Pedro Giffuni <pfg@FreeBSD.org> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Andrey Chernov <ache@freebsd.org> Subject: Re: svn commit: r278634 - head/lib/libc/gen Message-ID: <54DE2DE3.7050004@FreeBSD.org> In-Reply-To: <20150214032839.E3221@besplex.bde.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> <20150214032839.E3221@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 02/13/15 11:46, Bruce Evans wrote:
> 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.
>
Oops.. OK, I am pretty bad reading specifications.
>> ...
>>> 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.
>
OK. The patch passes tinderbox. The only missing thing is what to do
about arg (iff it has to be adjusted).
Pedro.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?54DE2DE3.7050004>