Here's a rework of the patch based on the comments so far.
It handles empty path elements properly, and it handles the munging of
errno properly. It uses a strbuf to avoid any path limitations (in
practice, I don't expect this to be much of an issue, but it matches
what glibc does. And this is the slow error-path anyway, so it's not a
big deal). And it has miscellaneous style fixes and comments.
No tests yet. I'll post some output on that in a minute.
---
diff --git a/cache.h b/cache.h
index e5e1aa4..59e1c44 100644
--- a/cache.h
+++ b/cache.h
@@ -1276,4 +1276,6 @@ extern struct startup_info *startup_info;
/* builtin/merge.c */
int checkout_fast_forward(const unsigned char *from, const unsigned char *to);
+int sane_execvp(const char *file, char *const argv[]);
+
#endif /* CACHE_H */
diff --git a/exec_cmd.c b/exec_cmd.c
index 171e841..125fa6f 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -134,7 +134,7 @@ int execv_git_cmd(const char **argv) {
trace_argv_printf(nargv, "trace: exec:");
/* execvp() can only ever return if it fails */
- execvp("git", (char **)nargv);
+ sane_execvp("git", (char **)nargv);
trace_printf("trace: exec failed: %s\n", strerror(errno));
diff --git a/run-command.c b/run-command.c
index 1db8abf..2b0c311 100644
--- a/run-command.c
+++ b/run-command.c
@@ -76,6 +76,63 @@ static inline void dup_devnull(int to)
}
#endif
+static int exists_in_PATH(const char *file)
+{
+ const char *p = getenv("PATH");
+ struct strbuf buf = STRBUF_INIT;
+
+ if (!p || !*p)
+ return 0;
+
+ while (1) {
+ const char *end = strchrnul(p, ':');
+
+ strbuf_reset(&buf);
+
+ /* POSIX specifies an empty entry as the current directory. */
+ if (end != p) {
+ strbuf_add(&buf, p, end - p);
+ strbuf_addch(&buf, '/');
+ }
+ strbuf_addstr(&buf, file);
+
+ if (!access(buf.buf, F_OK)) {
+ strbuf_release(&buf);
+ return 1;
+ }
+
+ if (!*end)
+ break;
+ p = end + 1;
+ }
+
+ strbuf_release(&buf);
+ return 0;
+}
+
+int sane_execvp(const char *file, char * const argv[])
+{
+ if (!execvp(file, argv))
+ return 0;
+
+ /*
+ * When a command can't be found because one of the directories
+ * listed in $PATH is unsearchable, execvp reports EACCES, but
+ * careful usability testing (read: analysis of occasional bug
+ * reports) reveals that "No such file or directory" is more
+ * intuitive.
+ *
+ * We avoid commands with "/", because execvp will not do $PATH
+ * lookups in that case.
+ *
+ * The reassignment of EACCES to errno looks like a no-op below,
+ * but we need to protect against exists_in_PATH overwriting errno.
+ */
+ if (errno == EACCES && !strchr(file, '/'))
+ errno = exists_in_PATH(file) ? EACCES : ENOENT;
+ return -1;
+}
+
static const char **prepare_shell_cmd(const char **argv)
{
int argc, nargc = 0;
@@ -114,7 +171,7 @@ static int execv_shell_cmd(const char **argv)
{
const char **nargv = prepare_shell_cmd(argv);
trace_argv_printf(nargv, "trace: exec:");
- execvp(nargv[0], (char **)nargv);
+ sane_execvp(nargv[0], (char **)nargv);
free(nargv);
return -1;
}
@@ -339,7 +396,7 @@ fail_pipe:
} else if (cmd->use_shell) {
execv_shell_cmd(cmd->argv);
} else {
- execvp(cmd->argv[0], (char *const*) cmd->argv);
+ sane_execvp(cmd->argv[0], (char *const*) cmd->argv);
}
if (errno == ENOENT) {
if (!cmd->silent_exec_failure)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html