# [PATCH] use only the PATH for exec'ing git commands 7 messages from 2007-10-22 to 2007-10-23. Participants: Scott R Parish, Johannes Schindelin, Alex Riesen, Eric Merritt, Scott Parish. Thread: https://gitlist.dev/t/10431 ## Scott R Parish, 2007-10-22 17:01 Subject: [PATCH] use only the PATH for exec'ing git commands Message-ID: <20071022170148.GB29642@srparish.net> URL: https://gitlist.dev/e/20071022170148.GB29642%40srparish.net ``` We need to correctly set up PATH for non-c based git commands. Since we already do this, we can just use that PATH and execvp, instead of looping over the paths with execve. This patch adds a setup_path() function to exec_cmd.c, which sets the PATH order correctly for our search order. execv_git_cmd() is stripped down to setting up argv and calling execvp(). git.c's main() only only needs to call setup_path(). Signed-off-by: Scott R Parish --- exec_cmd.c | 122 ++++++++++++++++++++++++++---------------------------------- exec_cmd.h | 1 + git.c | 43 +++------------------ 3 files changed, 61 insertions(+), 105 deletions(-) diff --git a/exec_cmd.c b/exec_cmd.c index 8b681d0..b154c24 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -29,85 +29,69 @@ const char *git_exec_path(void) return builtin_exec_path; } +static void add_path(struct strbuf *out, const char *path) +{ + if (path && strlen(path)) { + if (is_absolute_path(path)) + strbuf_addstr(out, path); + else + strbuf_addstr(out, make_absolute_path(path)); + + strbuf_addch(out, ':'); + } +} + +void setup_path(const char *cmd_path) +{ + const char *old_path = getenv("PATH"); + struct strbuf new_path; + + strbuf_init(&new_path, 0); + + add_path(&new_path, argv_exec_path); + add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT)); + add_path(&new_path, builtin_exec_path); + add_path(&new_path, cmd_path); + + if (old_path) + strbuf_addstr(&new_path, old_path); + else + strbuf_addstr(&new_path, "/usr/local/bin:/usr/bin:/bin"); + + setenv("PATH", new_path.buf, 1); + + strbuf_release(&new_path); +} + int execv_git_cmd(const char **argv) { - char git_command[PATH_MAX + 1]; - int i; - const char *paths[] = { argv_exec_path, - getenv(EXEC_PATH_ENVIRONMENT), - builtin_exec_path }; - - for (i = 0; i < ARRAY_SIZE(paths); ++i) { - size_t len; - int rc; - const char *exec_dir = paths[i]; - const char *tmp; - - if (!exec_dir || !*exec_dir) continue; - - if (*exec_dir != '/') { - if (!getcwd(git_command, sizeof(git_command))) { - fprintf(stderr, "git: cannot determine " - "current directory: %s\n", - strerror(errno)); - break; - } - len = strlen(git_command); - - /* Trivial cleanup */ - while (!prefixcmp(exec_dir, "./")) { - exec_dir += 2; - while (*exec_dir == '/') - exec_dir++; - } - - rc = snprintf(git_command + len, - sizeof(git_command) - len, "/%s", - exec_dir); - if (rc < 0 || rc >= sizeof(git_command) - len) { - fprintf(stderr, "git: command name given " - "is too long.\n"); - break; - } - } else { - if (strlen(exec_dir) + 1 > sizeof(git_command)) { - fprintf(stderr, "git: command name given " - "is too long.\n"); - break; - } - strcpy(git_command, exec_dir); - } - - len = strlen(git_command); - rc = snprintf(git_command + len, sizeof(git_command) - len, - "/git-%s", argv[0]); - if (rc < 0 || rc >= sizeof(git_command) - len) { - fprintf(stderr, - "git: command name given is too long.\n"); - break; - } + struct strbuf cmd; + const char *tmp; - /* argv[0] must be the git command, but the argv array - * belongs to the caller, and my be reused in - * subsequent loop iterations. Save argv[0] and - * restore it on error. - */ + strbuf_init(&cmd, 0); + strbuf_addf(&cmd, "git-%s", argv[0]); - tmp = argv[0]; - argv[0] = git_command; + /* argv[0] must be the git command, but the argv array + * belongs to the caller, and my be reused in + * subsequent loop iterations. Save argv[0] and + * restore it on error. + */ + tmp = argv[0]; + argv[0] = cmd.buf; - trace_argv_printf(argv, -1, "trace: exec:"); + trace_argv_printf(argv, -1, "trace: exec:"); - /* execve() can only ever return if it fails */ - execve(git_command, (char **)argv, environ); + /* execvp() can only ever return if it fails */ + execvp(cmd.buf, (char **)argv); - trace_printf("trace: exec failed: %s\n", strerror(errno)); + trace_printf("trace: exec failed: %s\n", strerror(errno)); - argv[0] = tmp; - } - return -1; + argv[0] = tmp; + strbuf_release(&cmd); + + return -1; } diff --git a/exec_cmd.h b/exec_cmd.h index da99287..a892355 100644 --- a/exec_cmd.h +++ b/exec_cmd.h @@ -3,6 +3,7 @@ extern void git_set_argv_exec_path(const char *exec_path); extern const char* git_exec_path(void); +extern void setup_path(const char *); extern int execv_git_cmd(const char **argv); /* NULL terminated */ extern int execl_git_cmd(const char *cmd, ...); diff --git a/git.c b/git.c index f659338..a639e42 100644 --- a/git.c +++ b/git.c @@ -6,28 +6,6 @@ const char git_usage_string[] = "git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]"; -static void prepend_to_path(const char *dir, int len) -{ - const char *old_path = getenv("PATH"); - char *path; - int path_len = len; - - if (!old_path) - old_path = "/usr/local/bin:/usr/bin:/bin"; - - path_len = len + strlen(old_path) + 1; - - path = xmalloc(path_len + 1); - - memcpy(path, dir, len); - path[len] = ':'; - memcpy(path + len + 1, old_path, path_len - len); - - setenv("PATH", path, 1); - - free(path); -} - static int handle_options(const char*** argv, int* argc, int* envchanged) { int handled = 0; @@ -403,7 +381,7 @@ int main(int argc, const char **argv) { const char *cmd = argv[0] ? argv[0] : "git-help"; char *slash = strrchr(cmd, '/'); - const char *exec_path = NULL; + const char *cmd_path = NULL; int done_alias = 0; /* @@ -413,10 +391,7 @@ int main(int argc, const char **argv) */ if (slash) { *slash++ = 0; - if (*cmd == '/') - exec_path = cmd; - else - exec_path = xstrdup(make_absolute_path(cmd)); + cmd_path = cmd; cmd = slash; } @@ -451,16 +426,12 @@ int main(int argc, const char **argv) cmd = argv[0]; /* - * We execute external git command via execv_git_cmd(), - * which looks at "--exec-path" option, GIT_EXEC_PATH - * environment, and $(gitexecdir) in Makefile while built, - * in this order. For scripted commands, we prepend - * the value of the exec_path variable to the PATH. + * We use PATH to find git commands, but we prepend some higher + * precidence paths: the "--exec-path" option, the GIT_EXEC_PATH + * environment, and the $(gitexecdir) from the Makefile at build + * time. */ - if (exec_path) - prepend_to_path(exec_path, strlen(exec_path)); - exec_path = git_exec_path(); - prepend_to_path(exec_path, strlen(exec_path)); + setup_path(cmd_path); while (1) { /* See if it's an internal command */ -- gitgui.0.8.4.11176.gd9205-dirty ``` ## Johannes Schindelin, 2007-10-22 17:44 Subject: Re: [PATCH] use only the PATH for exec'ing git commands Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0710221843590.25221%40racer.site In-Reply-To: <20071022170148.GB29642@srparish.net> ``` Hi, On Mon, 22 Oct 2007, Scott R Parish wrote: > 3 files changed, 61 insertions(+), 105 deletions(-) Nice. Ciao, Dscho ``` ## Alex Riesen, 2007-10-22 19:01 Subject: Re: [PATCH] use only the PATH for exec'ing git commands Message-ID: <20071022190102.GA23714@steel.home> URL: https://gitlist.dev/e/20071022190102.GA23714%40steel.home In-Reply-To: <20071022170148.GB29642@srparish.net> ``` Scott R Parish, Mon, Oct 22, 2007 19:01:48 +0200: > +static void add_path(struct strbuf *out, const char *path) > +{ > + if (path && strlen(path)) { > + if (is_absolute_path(path)) > + strbuf_addstr(out, path); > + else > + strbuf_addstr(out, make_absolute_path(path)); > + > + strbuf_addch(out, ':'); Shouldn't it break MingW32 native port? > + } > +} > + > +void setup_path(const char *cmd_path) > +{ > + const char *old_path = getenv("PATH"); > + struct strbuf new_path; > + > + strbuf_init(&new_path, 0); > + > + add_path(&new_path, argv_exec_path); > + add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT)); > + add_path(&new_path, builtin_exec_path); > + add_path(&new_path, cmd_path); > + trailing space > + if (old_path) > + strbuf_addstr(&new_path, old_path); > + else > + strbuf_addstr(&new_path, "/usr/local/bin:/usr/bin:/bin"); the default PATH is platform-dependent. Git is multi-platform. You should consider putting the path list somewhere in Makefile, config.mak or configure. > + > + setenv("PATH", new_path.buf, 1); ``` ## Eric Merritt, 2007-10-22 19:25 Subject: Re: [PATCH] use only the PATH for exec'ing git commands Message-ID: URL: https://gitlist.dev/e/f69c840c0710221225u5df73cf9x7f95ea3bd530c160%40mail.gmail.com In-Reply-To: <20071022190102.GA23714@steel.home> ``` > the default PATH is platform-dependent. Git is multi-platform. > You should consider putting the path list somewhere in Makefile, > config.mak or configure. This static configuration that you describe is one of the things this patch is designed to strip out. Compile time configuration breaks down completly if you don't deploy to the path defined when the system was compiled. Thats a problem. ``` ## Scott Parish, 2007-10-23 04:08 Subject: [PATCH trailing ws fixed] use only the PATH for exec'ing git commands Message-ID: <20071023040844.GQ16291@srparish.net> URL: https://gitlist.dev/e/20071023040844.GQ16291%40srparish.net In-Reply-To: <20071022190102.GA23714@steel.home> ``` We need to correctly set up PATH for non-c based git commands. Since we already do this, we can just use that PATH and execvp, instead of looping over the paths with execve. This patch adds a setup_path() function to exec_cmd.c, which sets the PATH order correctly for our search order. execv_git_cmd() is stripped down to setting up argv and calling execvp(). git.c's main() only only needs to call setup_path(). Signed-off-by: Scott R Parish --- exec_cmd.c | 121 ++++++++++++++++++++++++++---------------------------------- exec_cmd.h | 1 + git.c | 43 +++------------------ 3 files changed, 60 insertions(+), 105 deletions(-) diff --git a/exec_cmd.c b/exec_cmd.c index 8b681d0..c228dbf 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -29,85 +29,68 @@ const char *git_exec_path(void) return builtin_exec_path; } +static void add_path(struct strbuf *out, const char *path) +{ + if (path && strlen(path)) { + if (is_absolute_path(path)) + strbuf_addstr(out, path); + else + strbuf_addstr(out, make_absolute_path(path)); + + strbuf_addch(out, ':'); + } +} + +void setup_path(const char *cmd_path) +{ + const char *old_path = getenv("PATH"); + struct strbuf new_path; + + strbuf_init(&new_path, 0); + + add_path(&new_path, argv_exec_path); + add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT)); + add_path(&new_path, builtin_exec_path); + add_path(&new_path, cmd_path); + + if (old_path) + strbuf_addstr(&new_path, old_path); + else + strbuf_addstr(&new_path, "/usr/local/bin:/usr/bin:/bin"); + + setenv("PATH", new_path.buf, 1); + + strbuf_release(&new_path); +} int execv_git_cmd(const char **argv) { - char git_command[PATH_MAX + 1]; - int i; - const char *paths[] = { argv_exec_path, - getenv(EXEC_PATH_ENVIRONMENT), - builtin_exec_path }; - - for (i = 0; i < ARRAY_SIZE(paths); ++i) { - size_t len; - int rc; - const char *exec_dir = paths[i]; - const char *tmp; - - if (!exec_dir || !*exec_dir) continue; - - if (*exec_dir != '/') { - if (!getcwd(git_command, sizeof(git_command))) { - fprintf(stderr, "git: cannot determine " - "current directory: %s\n", - strerror(errno)); - break; - } - len = strlen(git_command); - - /* Trivial cleanup */ - while (!prefixcmp(exec_dir, "./")) { - exec_dir += 2; - while (*exec_dir == '/') - exec_dir++; - } - - rc = snprintf(git_command + len, - sizeof(git_command) - len, "/%s", - exec_dir); - if (rc < 0 || rc >= sizeof(git_command) - len) { - fprintf(stderr, "git: command name given " - "is too long.\n"); - break; - } - } else { - if (strlen(exec_dir) + 1 > sizeof(git_command)) { - fprintf(stderr, "git: command name given " - "is too long.\n"); - break; - } - strcpy(git_command, exec_dir); - } - - len = strlen(git_command); - rc = snprintf(git_command + len, sizeof(git_command) - len, - "/git-%s", argv[0]); - if (rc < 0 || rc >= sizeof(git_command) - len) { - fprintf(stderr, - "git: command name given is too long.\n"); - break; - } + struct strbuf cmd; + const char *tmp; - /* argv[0] must be the git command, but the argv array - * belongs to the caller, and my be reused in - * subsequent loop iterations. Save argv[0] and - * restore it on error. - */ + strbuf_init(&cmd, 0); + strbuf_addf(&cmd, "git-%s", argv[0]); - tmp = argv[0]; - argv[0] = git_command; + /* argv[0] must be the git command, but the argv array + * belongs to the caller, and my be reused in + * subsequent loop iterations. Save argv[0] and + * restore it on error. + */ + tmp = argv[0]; + argv[0] = cmd.buf; - trace_argv_printf(argv, -1, "trace: exec:"); + trace_argv_printf(argv, -1, "trace: exec:"); - /* execve() can only ever return if it fails */ - execve(git_command, (char **)argv, environ); + /* execvp() can only ever return if it fails */ + execvp(cmd.buf, (char **)argv); - trace_printf("trace: exec failed: %s\n", strerror(errno)); + trace_printf("trace: exec failed: %s\n", strerror(errno)); - argv[0] = tmp; - } - return -1; + argv[0] = tmp; + strbuf_release(&cmd); + + return -1; } diff --git a/exec_cmd.h b/exec_cmd.h index da99287..a892355 100644 --- a/exec_cmd.h +++ b/exec_cmd.h @@ -3,6 +3,7 @@ extern void git_set_argv_exec_path(const char *exec_path); extern const char* git_exec_path(void); +extern void setup_path(const char *); extern int execv_git_cmd(const char **argv); /* NULL terminated */ extern int execl_git_cmd(const char *cmd, ...); diff --git a/git.c b/git.c index f659338..a639e42 100644 --- a/git.c +++ b/git.c @@ -6,28 +6,6 @@ const char git_usage_string[] = "git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]"; -static void prepend_to_path(const char *dir, int len) -{ - const char *old_path = getenv("PATH"); - char *path; - int path_len = len; - - if (!old_path) - old_path = "/usr/local/bin:/usr/bin:/bin"; - - path_len = len + strlen(old_path) + 1; - - path = xmalloc(path_len + 1); - - memcpy(path, dir, len); - path[len] = ':'; - memcpy(path + len + 1, old_path, path_len - len); - - setenv("PATH", path, 1); - - free(path); -} - static int handle_options(const char*** argv, int* argc, int* envchanged) { int handled = 0; @@ -403,7 +381,7 @@ int main(int argc, const char **argv) { const char *cmd = argv[0] ? argv[0] : "git-help"; char *slash = strrchr(cmd, '/'); - const char *exec_path = NULL; + const char *cmd_path = NULL; int done_alias = 0; /* @@ -413,10 +391,7 @@ int main(int argc, const char **argv) */ if (slash) { *slash++ = 0; - if (*cmd == '/') - exec_path = cmd; - else - exec_path = xstrdup(make_absolute_path(cmd)); + cmd_path = cmd; cmd = slash; } @@ -451,16 +426,12 @@ int main(int argc, const char **argv) cmd = argv[0]; /* - * We execute external git command via execv_git_cmd(), - * which looks at "--exec-path" option, GIT_EXEC_PATH - * environment, and $(gitexecdir) in Makefile while built, - * in this order. For scripted commands, we prepend - * the value of the exec_path variable to the PATH. + * We use PATH to find git commands, but we prepend some higher + * precidence paths: the "--exec-path" option, the GIT_EXEC_PATH + * environment, and the $(gitexecdir) from the Makefile at build + * time. */ - if (exec_path) - prepend_to_path(exec_path, strlen(exec_path)); - exec_path = git_exec_path(); - prepend_to_path(exec_path, strlen(exec_path)); + setup_path(cmd_path); while (1) { /* See if it's an internal command */ -- gitgui.0.8.4.11176.gd9205-dirty ``` ## Alex Riesen, 2007-10-23 21:12 Subject: Re: [PATCH trailing ws fixed] use only the PATH for exec'ing git commands Message-ID: <20071023211202.GA729@steel.home> URL: https://gitlist.dev/e/20071023211202.GA729%40steel.home In-Reply-To: <20071023040844.GQ16291@srparish.net> ``` Scott Parish, Tue, Oct 23, 2007 06:08:45 +0200: > We need to correctly set up PATH for non-c based git commands. Since we > already do this, we can just use that PATH and execvp, instead of looping > over the paths with execve. > > This patch adds a setup_path() function to exec_cmd.c, which sets > the PATH order correctly for our search order. execv_git_cmd() is > stripped down to setting up argv and calling execvp(). git.c's main() > only only needs to call setup_path(). > > Signed-off-by: Scott R Parish Acked-by: Alex Riesen Don't forget to mention it needs the patch from Johannes re "deducing exec_path from calls to git with a relative path" and your "current_exec_path"-patch. Took me a while to figure these out... ``` ## Scott R Parish, 2007-10-23 21:27 Subject: Re: [PATCH trailing ws fixed] use only the PATH for exec'ing gitcommands Message-ID: <1193174862.v1.fusewebmail-240137@f> URL: https://gitlist.dev/e/1193174862.v1.fusewebmail-240137%40f In-Reply-To: <20071023211202.GA729@steel.home> ``` > Acked-by: Alex Riesen > > Don't forget to mention it needs the patch from Johannes re "deducing > exec_path from calls to git with a relative path" and your > "current_exec_path"-patch. Took me a while to figure these out... Sorry about that! I was basing my patches off the head of spearce, as it sounded like Junio was going to pick that stuff up. sRp ```