Re: setenv if value=NULL, what say standard? Bug?

[Jens Gustedt <jens.gustedt@inria.fr>]

[-- Attachment #1: Type: text/plain, Size: 3793 bytes --]

Hello

Am Mittwoch, den 22.04.2015, 22:15 -0400 schrieb Rich Felker:
> On Wed, Apr 22, 2015 at 09:26:57PM -0400, Jean-Marc Pigeon wrote:
> > > I think the only safe conclusion is that the application is
> > > incorrect and should ensure that setenv() is never called with a
> > > NULL value.
> > > 
> > Checked glibc, My understanding, it set something as
> > "name="
> > in the environment, so the variable is present but
> > value is "empty"i (top application to decide what to do).
> > uclibc does something similar (as far I can tell looking
> > at source code)..
> > 
> > 
> > The application is not careful enough, but not incorrect as such.
> 
> It's definitely incorrect. It's doing something that invokes undefined
> behavior.

You probably mean it *has* undefined behavior. UB is nothing that can
be invoked.

Yes, it actually has two bugs in the case that was the starting point
of this thread. It has to calls to libc functions where it doesn't
check the return values.

> > Note: we may have tons of applications with the same problem.
> > if we keep musl setenv like that, musl will be seen as quite unreliable.
> 
> No, actually glibc is fixing this bug (maybe they already did). See
> the thread beginning here:
> 
> https://sourceware.org/ml/libc-alpha/2015-03/threads.html#00449
> 
> My understanding is that glibc is planning to do, or already does in
> the latest version, exactly what musl is doing.
> 
> > If this situation is indeed UB, there is 2 options for musl:
> > 1) Swallow the problem nicely... as glibc and uclibc does.
> > 2) Report an error.. EINVAL? (and document it in manual)
> > 
> > Crashing at "libc" level is not an option.
> 
> I can see how it might seem like that at first, but crashing is
> actually the best possible behavior. Options 1 and 2 cover up a
> potentially serious bug -- it's not clear what the application was
> trying to do, most likely nobody even thought about what they were
> trying to do, and even if they did have something in mind it's not
> reliable or portable. The glibc wiki has some text taken from text I
> wrote on the topic (copied from a stack overflow answer I gave) here:
> 
> https://sourceware.org/glibc/wiki/Style_and_Conventions#Invalid_pointers
> 
> Specifically it covers why returning an error is not a good idea.

I see your point, but I would go a bit more moderately and more
pragmatically with it.

First of all UB is what it is, a specific standard (here POSIX)
doesn't impose any form of behavior. So an implementation may extend
the behavior as it pleases. (Otherwise these standards have a saying
"it is unspecified whether ... or not ...")

Now, failing early is certainly a good property when we can expect
just that; any application that uses the call in that way *will* in
fact fail early. This is particularly important in code that otherwise
will have a performance penalty for doing checks.

Another acceptable strategy, IMPOV, is to forward errors where this is
easy to do and the check doesn't impose an unacceptable penalty. The
application then can handle the error (or not).

setenv is certainly borderline. Code for which it is performance
critical is almost certainly broken in many ways, and on the other
hand failures in the way we have seen here can be rare and late.

So I would have a small preference for being nice: do nothing to the
environment and return an error.

Just my 2 ¢

Jens

-- 
:: INRIA Nancy Grand Est ::: Camus ::::::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]