Review request for 5049299

[Martin Buchholz]

On Mon, Jun 8, 2009 at 07:08, Michael McMahon <Michael.McMahon at sun.com>wrote:

> That's fine Martin. We can do it that way.
>
> Do you really need to #include <sys/syscall.h>?
> As far as I can see clone() only requires <sched.h>
>

You're right.  I removed #include of syscall.h.
I think I picked it up from some man page somewhere.


>
> When you allocate the clone stack for the child
> the memory is byte aligned. Is this ok for Linux or should stacks
> be aligned on larger boundaries?
>

Good question.  The man page is silent on the matter.

I ensure that the stack pointer is as aligned as the return from malloc(),
which is supposed to be suitable for any C object.  I suspect that if there
are
more stringent requirements, then the kernel itself will adjust the
alignment.
Let's keep the current code unless we know of a reason not to.



>
> 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().
 */

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

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>> 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/20090608/23357574/attachment.html>