Re: [PATCH v6 04/11] run-command: use the async-signal-safe execv instead of execvp
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 17, 2017, 02:15 UTC
- Message-ID
- <xmqqfug4z0s0.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170419231327.49895-5-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
Show 13 quoted lines
> @@ -238,6 +238,12 @@ static void prepare_cmd(struct argv_array *out, const struct child_process *cmd)
> if (!cmd->argv[0])
> die("BUG: command is empty");
>
> + /*
> + * Add SHELL_PATH so in the event exec fails with ENOEXEC we can
> + * attempt to interpret the command with 'sh'.
> + */
> + argv_array_push(out, SHELL_PATH);
> +
> if (cmd->git_cmd) {
> argv_array_push(out, "git");
> argv_array_pushv(out, cmd->argv);So, given "cat-file", "-t", "HEAD" with cmd->git_cmd == TRUE, this will now prepare ("/bin/sh", "git", "cat-file", "-t", "HEAD", NULL) in the argv-array, and then ...
Show 18 quoted lines
> @@ -246,6 +252,20 @@ static void prepare_cmd(struct argv_array *out, const struct child_process *cmd)
> } else {
> argv_array_pushv(out, cmd->argv);
> }
> +
> + /*
> + * If there are no '/' characters in the command then perform a path
> + * lookup and use the resolved path as the command to exec. If there
> + * are no '/' characters or if the command wasn't found in the path,
> + * have exec attempt to invoke the command directly.
> + */
> + if (!strchr(out->argv[1], '/')) {
> + char *program = locate_in_PATH(out->argv[1]);
> + if (program) {
> + free((char *)out->argv[1]);
> + out->argv[1] = program;
> + }
> + }... turn the first element from "git" to "/usr/bin/git", i.e.
("/bin/sh", "/usr/bin/git", "cat-file", "-t", "HEAD", NULL)which ...
Show 14 quoted lines
> #endif > > @@ -445,7 +465,15 @@ int start_command(struct child_process *cmd) > } > } > > - sane_execvp(argv.argv[0], (char *const *) argv.argv); > + /* > + * Attempt to exec using the command and arguments starting at > + * argv.argv[1]. argv.argv[0] contains SHELL_PATH which will > + * be used in the event exec failed with ENOEXEC at which point > + * we will try to interpret the command using 'sh'. > + */ > + execv(argv.argv[1], (char *const *) argv.argv + 1);
... first is given without the leading "/bin/sh", as the end-user intended (sort of), but if it fails
> + if (errno == ENOEXEC) > + execv(argv.argv[0], (char *const *) argv.argv);
"/bin/sh" tries to run "/usr/bin/git" that was not executable (well, the one in "usr/bin/" would have +x bit, but let's pretend that we are trying to run one from bin-wrappers/ and somehow forgot +x bit)?
I think all of that is sensible, but there is one "huh?" I can't figure out. Typically we do "sh -c git cat-file -t HEAD" but this lacks the "-c" (cf. the original prepare_shell_cmd()); why do we not need it in this case?
Thanks.