602140 - posix_spawnp for nsIProcess on Mac OS X should pass the environment

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Robert Strong (they/them - no direct email)]

+++ This bug was initially created as a clone of Bug #600711 +++

Since it does this for other platforms.

Comment 1

[(not currently active) Ted Mielczarek]

Josh and I discussed this when he was writing the patch, and came to the conclusion that the previous nsIProcess implementation did not preserve the environment, so he went for the compatible approach.

Comment 2

[Benjamin Smedberg]

"preserve" the environment? Child processes inherit the environment by default, in general. Does posix_spawnp not have this behavior?

Comment 3

[(not currently active) Ted Mielczarek]

posix_spawnp takes an envp argument:
http://developer.apple.com/library/mac/#documentation/Darwin/Reference/ManPages/man2/posix_spawnp.2.html
just like execve:
http://www.opengroup.org/onlinepubs/009695399/functions/exec.html

according to POSIX:
"The argument envp is an array of character pointers to null-terminated strings. These strings shall constitute the environment for the new process image."
"For those forms not containing an envp pointer ( execl(), execv(), execlp(), and execvp()), the environment for the new process image shall be taken from the external variable environ in the calling process."

I guess the not-explicitly-stated thing there is that if the function you're calling takes an envp argument and you pass NULL, the new process has an empty environment. If you want to preserve the environment you have to explicitly pass the parent's environment.

Comment 4

[Benjamin Smedberg]

We should definitely be cloning the environment by passing 'environ', not creating an empty environment. I've verified that passing NULL doesn't clone the environment as I expected it would.

Comment 5

[Josh Aas]

Attachment: fix v1.0

Comment 6

[Benjamin Smedberg]

Comment on attachment 481215 [details] [diff] [review]
fix v1.0

Is _NSGetEnviron better than environ? It seems really strange that it would ever return NULL. I really want this to just be

posix_spawnp(..., environ) or posix_spawnp(..., *_NSGetEnviron())

FWIW, in my little test app I had to declare 'extern "C" extern char **environ;'.

Comment 7

[Josh Aas]

I think usage of "environ" vs. "_NSGetEnviron" comes down to this, in the Darwin "environ" docs:

"Shared libraries and bundles don't have direct access to environ, which is only available to the loader ld(1) when a complete program is being linked.  The environment routines can still be used, but if direct access to environ is needed, the _NSGetEnviron() routine, defined in <crt_externs.h>, can be used to retrieve the address of environ at runtime."

I don't know if "_NSGetEnviron" can return NULL, I was just being safe when I wrote the NULL check.

Comment 8

[Benjamin Smedberg]

That makes sense. That implies it can't be null, so let's just do *_NSGetEnviron() in the posix call.

Comment 9

[Josh Aas]

Attachment: fix v1.1

Comment 10

[Josh Aas]

pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/ea0dcfd46c61