Review request for 5049299

Martin Buchholz martinrb at google.com
Tue Jun 9 15:51:04 UTC 2009


On Tue, Jun 9, 2009 at 06:56, Michael McMahon <Michael.McMahon at sun.com>wrote:

> Martin Buchholz wrote:
>
>>
>>
>>    Also, I don't follow why we need the
>>     execve_as_traditional_shell_script()
>>    function. Can you explain the reason for that?
>>
>>
>> I think my comment for that function explains it fairly well.
>>
>> /**
>>  * Exec FILE as a traditional Bourne shell script (i.e. one without #!).
>>  * If we could do it over again, we would probably not support such an
>> ancient
>>  * misfeature, but compatibility wins over sanity.  The original support
>> for
>>  * this was imported accidentally from execvp().
>>  */
>>
>>  Actually, I was really wondering why is this code needed now?
> What has it to do with the specifics of converting fork()+exec()
> to clone()+exec()
>

In the old implementation, we used the strategy of
fork+mutate environ+execvp
and we got the traditional shell script semantics
from our use of execvp.
Since environ is now a global shared-with-parent variable,
we can't mutate it any more, therefore can't use execvp,
and must implement the missing execvpe ourselves.

Now, strictly speaking, execvp's traditional shell script semantics
is an unspecified implementation detail of execvp;
on other Unix platforms we might not need this.
We can leave this to, e.g. the BSD porters reading this,
to add some #ifdefs.

(It would also be good if you ran the updated tests on
Solaris with the old implementation to confirm my understanding)

The other non-portable implementation detail
we need to deal with is the
default value of PATH if undefined in the environment,
but we had already been doing that.

Martin


>
> Thanks,
> Michael.
>
>> The tests I added also pass on the older implementation,
>> so execve_as_traditional_shell_script() prevents a regression.
>> We always supported "traditional shell scripts" - we just didn't know it.
>>
>> ---
>>
>> I updated the public version of the patch at:
>> http://cr.openjdk.java.net/~martin/clone-exec<http://cr.openjdk.java.net/%7Emartin/clone-exec><
>> http://cr.openjdk.java.net/%7Emartin/clone-exec>
>>
>>
>> Martin
>>
>>
>>
>>    Thanks,
>>    Michael.
>>
>>    Martin Buchholz wrote:
>>
>>        Michael,
>>
>>        I think the best way to handle the coordination is in two steps.
>>        I'd like to get my Linux-clone changes in first (you should
>>        review,
>>        I will commit)
>>        and then we switch hats and I will review your Solaris changes.
>>        It seems best to do this in two steps: to better place blame when
>>        it breaks (this is very tricky stuff to get right).
>>        If you agree, please review my posted changes.
>>
>>        Aside: Instead of griping about the missing execvpe,
>>        I filed a bug against glibc, and was surprised to find
>>        that Ulrich Drepper had implemented it a couple of days later.
>>        It will probably be in glibc-2.11.  Perhaps in 5 years we can
>>        use it ourselves...).  Thanks, Uli!
>>
>>        Martin
>>
>>        On Tue, Jun 2, 2009 at 07:29, Michael McMahon
>>        <Michael.McMahon at sun.com <mailto:Michael.McMahon at sun.com>
>>        <mailto:Michael.McMahon at sun.com
>>        <mailto:Michael.McMahon at sun.com>>> wrote:
>>
>>           Martin,
>>
>>           I had done something similar with clone & exec for Linux, but
>>           hadn't got round to testing it.
>>           So, it seems reasonable to take yours. Do you want to send
>>        me your
>>           updated versions of
>>           process_md.c and the test? I can take care of the merge
>>        with the
>>           Solaris code.
>>
>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/core-libs-dev/attachments/20090609/0a5a88df/attachment.html>


More information about the core-libs-dev mailing list