On Tue, May 16, 2017 at 10:23 AM, Jeff King <peff@xxxxxxxx> wrote:
>
> I think the logic here would be more like:
>
> 1. During prepare_shell_cmd(), even if we optimize out the shell call,
> still prepare a fallback argv (since we can't allocate memory
> post-fork).
>
> 2. In the forked child, if we get ENOENT from exec and cmd->use_shell
> is set, then exec the fallback shell argv instead. Propagate its
> results, even if it's 127.
>
> That still means we'd prefer a $PATH copy of a command to its shell
> builtin variant, but that can't be helped (and I kind of doubt anybody
> would care too much).
I think it would be better to just
(a) get rid of the magic strcspn() entirely
(b) make the 'can we optimize this' test be simply just looking up
'argv[0]' in $PATH
Hmm? If you have executables with special characters in them in your
PATH, you have bigger issues.
Also, if people really want to optimize the code that executes an
external program (whether in shell or directly), I think it might be
worth it to look at replacing the "fork()" with a "vfork()".
Something like this
- cmd->pid = fork();
+ cmd->pid = (cmd->git_cmd || cmd->env) ? fork() : vfork();
might work (the native git_cmd case needs a real fork, and if we
change the environment variables we need it too, but the other cases
look like they might work with vfork()).
Using vfork() can be hugely more efficient, because you don't have the
extra page table copies and teardown, but also avoid a lot of possible
copy-on-write faults.
Linus