This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2 3/3] posix: New Linux posix_spawn{p} implementation
- From: Rasmus Villemoes <rv at rasmusvillemoes dot dk>
- To: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- Cc: Joseph Myers <joseph at codesourcery dot com>, libc-alpha at sourceware dot org
- Date: Wed, 14 Sep 2016 20:58:23 +0200
- Subject: Re: [PATCH v2 3/3] posix: New Linux posix_spawn{p} implementation
- Authentication-results: sourceware.org; auth=none
- References: <1454343665-1706-1-git-send-email-adhemerval.zanella@linaro.org> <1454343665-1706-4-git-send-email-adhemerval.zanella@linaro.org> <87mvjsprqb.fsf@rasmusvillemoes.dk> <alpine.DEB.2.20.1608312206510.24256@digraph.polyomino.org.uk> <CAKwiHFiuZVLX+S8b7OZxJfcdvZ2mjWV6p0CAnQRJHOfHcmn-HQ@mail.gmail.com> <c3d08ab5-dd2d-1875-995f-f00c2a1cf75c@linaro.org>
On Wed, Sep 14 2016, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> I think patch is ok and fixes the issues you noted about using the pipe2
> call to signal the execv issue. It just have one remark about it below.
>
>
>> @@ -280,14 +267,12 @@ __spawni_child (void *arguments)
>> (2.15). */
>> maybe_script_execute (args);
>>
>> - ret = -errno;
>> -
>> fail:
>> - /* Since sizeof errno < PIPE_BUF, the write is atomic. */
>> - ret = -ret;
>> - if (ret)
>> - while (write_not_cancel (p, &ret, sizeof ret) < 0)
>> - continue;
>> + /* errno should have an appropriate non-zero value, but make sure
>> + that's the case so that our parent knows we failed to
>> + exec. There's no EUNKNOWN or EINTERNALBUG, so we use a value
>> + which is clearly bogus. */
>> + args->err = errno ? : EHOSTDOWN;
>> _exit (SPAWN_ERROR);
>> }
>
> I would prefer an assert call here to ensure errno is non zero for
> failure case instead of reporting a bogus errno to program. Since
> this unexpected issue is either something wrong being reported from
> kernel or an underlying bug it would be better to fail at once than
> instead to document on manuals that this is potentially an unknown
> issue.
But asserting/aborting in the child doesn't really solve the problem; we
still need to write some non-zero value for the parent to pick up once
we're gone. We could of course write -1 to indicate this really
exceptional situation, but that still leaves deciding how to handle that
in the parent. IMO an assert/abort is a little too harsh, but then the
parent has to return _some_ error code to its caller.
Rasmus