From: Frans Klaver <fransklaver@gmail.com>
Date: Thu, 29 Mar 2012 17:21:49 GMT
Subject: Re: [PATCH 2/2] git: continue alias lookup on EACCES errors
Message-ID: <op.wbxzunjh0aolir@keputer>
In-Reply-To: <20120329171525.GB12318@sigill.intra.peff.net>

On Thu, 29 Mar 2012 19:15:25 +0200, Jeff King <peff@peff.net> wrote:

> On Thu, Mar 29, 2012 at 01:16:47PM +0200, Frans Klaver wrote:
>
>> > Yes, we can differentiate after the fact. Though I think it ends up
>> > being almost the same code as just implementing execvp in the first
>> > place.
>>
>> It will, but doesn't stock execv*() also provide access to shell
>> builtins? If that's the case then I wouldn't be bothered by the extra
>> bit of code we need to understand what execvp has been doing. I think
>> it would be sane to keep sane_execvp a wrapper instead of a
>> reimplementation.
>
> No, definitely not. Handling builtins is the responsibility of the
> shell, not of execvp. It is responsible for falling back to "/bin/sh
> $file" if execve returns ENOEXEC.
>
> Anyway, I think the last round I posted is good enough. It is
> approaching execvp in complexity, but it is still a little bit simpler.
> And because it's on the error code path, if we are incompatible the
> worst thing we can screw up is the error message, not the actual exec.

Good. In that case I think this last looks good, indeed.

Frans