svn commit: r278634 - head/lib/libc/gen

[Pedro Giffuni]

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.