From 8dba1e634af1d973a47fca616a63e7062a256790 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 30 Dec 2009 05:53:16 -0500 Subject: run-command: add "use shell" option Many callsites run "sh -c $CMD" to run $CMD. We can make it a little simpler for them by factoring out the munging of argv. For simple cases with no arguments, this doesn't help much, but: 1. For cases with arguments, we save the caller from having to build the appropriate shell snippet. 2. We can later optimize to avoid the shell when there are no metacharacters in the program. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- run-command.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ run-command.h | 2 ++ 2 files changed, 49 insertions(+) diff --git a/run-command.c b/run-command.c index cf2d8f7fae..b2bdcfda3d 100644 --- a/run-command.c +++ b/run-command.c @@ -15,6 +15,48 @@ static inline void dup_devnull(int to) close(fd); } +static const char **prepare_shell_cmd(const char **argv) +{ + int argc, nargc = 0; + const char **nargv; + + for (argc = 0; argv[argc]; argc++) + ; /* just counting */ + /* +1 for NULL, +3 for "sh -c" plus extra $0 */ + nargv = xmalloc(sizeof(*nargv) * (argc + 1 + 3)); + + if (argc < 1) + die("BUG: shell command is empty"); + + nargv[nargc++] = "sh"; + nargv[nargc++] = "-c"; + + if (argc < 2) + nargv[nargc++] = argv[0]; + else { + struct strbuf arg0 = STRBUF_INIT; + strbuf_addf(&arg0, "%s \"$@\"", argv[0]); + nargv[nargc++] = strbuf_detach(&arg0, NULL); + } + + for (argc = 0; argv[argc]; argc++) + nargv[nargc++] = argv[argc]; + nargv[nargc] = NULL; + + return nargv; +} + +#ifndef WIN32 +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); + free(nargv); + return -1; +} +#endif + int start_command(struct child_process *cmd) { int need_in, need_out, need_err; @@ -123,6 +165,8 @@ fail_pipe: cmd->preexec_cb(); if (cmd->git_cmd) { execv_git_cmd(cmd->argv); + } else if (cmd->use_shell) { + execv_shell_cmd(cmd->argv); } else { execvp(cmd->argv[0], (char *const*) cmd->argv); } @@ -179,6 +223,8 @@ fail_pipe: if (cmd->git_cmd) { cmd->argv = prepare_git_cmd(cmd->argv); + } else if (cmd->use_shell) { + cmd->argv = prepare_shell_cmd(cmd->argv); } cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, env); @@ -297,6 +343,7 @@ static void prepare_run_command_v_opt(struct child_process *cmd, cmd->git_cmd = opt & RUN_GIT_CMD ? 1 : 0; cmd->stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0; cmd->silent_exec_failure = opt & RUN_SILENT_EXEC_FAILURE ? 1 : 0; + cmd->use_shell = opt & RUN_USING_SHELL ? 1 : 0; } int run_command_v_opt(const char **argv, int opt) diff --git a/run-command.h b/run-command.h index fb342090e3..967ba8cc09 100644 --- a/run-command.h +++ b/run-command.h @@ -33,6 +33,7 @@ struct child_process { unsigned git_cmd:1; /* if this is to be git sub-command */ unsigned silent_exec_failure:1; unsigned stdout_to_stderr:1; + unsigned use_shell:1; void (*preexec_cb)(void); }; @@ -46,6 +47,7 @@ extern int run_hook(const char *index_file, const char *name, ...); #define RUN_GIT_CMD 2 /*If this is to be git sub-command */ #define RUN_COMMAND_STDOUT_TO_STDERR 4 #define RUN_SILENT_EXEC_FAILURE 8 +#define RUN_USING_SHELL 16 int run_command_v_opt(const char **argv, int opt); /* -- cgit v1.2.3 From fa7151a61edf585abdbb3d2831e7992bf3925671 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 1 Jan 2010 23:14:06 +0100 Subject: t0021: use $SHELL_PATH for the filter script On Windows, we need the shbang line to correctly invoke shell scripts via a POSIX shell, except when the script is invoked via 'sh -c' because sh (a bash) does "the right thing". But the clean and smudge filters will not always be invoked via 'sh -c'; to futureproof, we should mark the the one in t0021-conversion with #!$SHELL_PATH. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t0021-conversion.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh index 8fc39d77ce..6cb8d60ea2 100755 --- a/t/t0021-conversion.sh +++ b/t/t0021-conversion.sh @@ -4,7 +4,8 @@ test_description='blob conversion via gitattributes' . ./test-lib.sh -cat <<\EOF >rot13.sh +cat <rot13.sh +#!$SHELL_PATH tr \ 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ' \ 'nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM' -- cgit v1.2.3 From ac0ba18df0c58decfb128336bab51a172c77abc0 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 30 Dec 2009 05:53:57 -0500 Subject: run-command: convert simple callsites to use_shell Now that we have the use_shell feature, these callsites can all be converted with small changes. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- convert.c | 3 ++- imap-send.c | 8 ++------ ll-merge.c | 6 +++--- pager.c | 5 +++-- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/convert.c b/convert.c index 491e7141b4..950b1f9840 100644 --- a/convert.c +++ b/convert.c @@ -249,10 +249,11 @@ static int filter_buffer(int fd, void *data) struct child_process child_process; struct filter_params *params = (struct filter_params *)data; int write_err, status; - const char *argv[] = { "sh", "-c", params->cmd, NULL }; + const char *argv[] = { params->cmd, NULL }; memset(&child_process, 0, sizeof(child_process)); child_process.argv = argv; + child_process.use_shell = 1; child_process.in = -1; child_process.out = fd; diff --git a/imap-send.c b/imap-send.c index de8114bac0..51f371ba9f 100644 --- a/imap-send.c +++ b/imap-send.c @@ -965,17 +965,13 @@ static struct store *imap_open_store(struct imap_server_conf *srvc) /* open connection to IMAP server */ if (srvc->tunnel) { - const char *argv[4]; + const char *argv[] = { srvc->tunnel, NULL }; struct child_process tunnel = {0}; imap_info("Starting tunnel '%s'... ", srvc->tunnel); - argv[0] = "sh"; - argv[1] = "-c"; - argv[2] = srvc->tunnel; - argv[3] = NULL; - tunnel.argv = argv; + tunnel.use_shell = 1; tunnel.in = -1; tunnel.out = -1; if (start_command(&tunnel)) diff --git a/ll-merge.c b/ll-merge.c index 2d6b6d6cb1..18511e281f 100644 --- a/ll-merge.c +++ b/ll-merge.c @@ -175,7 +175,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn, { "B", temp[2] }, { NULL } }; - const char *args[] = { "sh", "-c", NULL, NULL }; + const char *args[] = { NULL, NULL }; int status, fd, i; struct stat st; @@ -190,8 +190,8 @@ static int ll_ext_merge(const struct ll_merge_driver *fn, strbuf_expand(&cmd, fn->cmdline, strbuf_expand_dict_cb, &dict); - args[2] = cmd.buf; - status = run_command_v_opt(args, 0); + args[0] = cmd.buf; + status = run_command_v_opt(args, RUN_USING_SHELL); fd = open(temp[1], O_RDONLY); if (fd < 0) goto bad; diff --git a/pager.c b/pager.c index 92c03f654a..2c7e8ecb3c 100644 --- a/pager.c +++ b/pager.c @@ -28,7 +28,7 @@ static void pager_preexec(void) } #endif -static const char *pager_argv[] = { "sh", "-c", NULL, NULL }; +static const char *pager_argv[] = { NULL, NULL }; static struct child_process pager_process; static void wait_for_pager(void) @@ -81,7 +81,8 @@ void setup_pager(void) spawned_pager = 1; /* means we are emitting to terminal */ /* spawn the pager */ - pager_argv[2] = pager; + pager_argv[0] = pager; + pager_process.use_shell = 1; pager_process.argv = pager_argv; pager_process.in = -1; if (!getenv("LESS")) { -- cgit v1.2.3 From f445644fd28d31a828731a618e9a9c79be89efd2 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 30 Dec 2009 05:55:36 -0500 Subject: run-command: optimize out useless shell calls If there are no metacharacters in the program to be run, we can just skip running the shell entirely and directly exec the program. The metacharacter test is pulled verbatim from launch_editor, which already implements this optimization. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- run-command.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/run-command.c b/run-command.c index b2bdcfda3d..47ced570bd 100644 --- a/run-command.c +++ b/run-command.c @@ -28,15 +28,17 @@ static const char **prepare_shell_cmd(const char **argv) if (argc < 1) die("BUG: shell command is empty"); - nargv[nargc++] = "sh"; - nargv[nargc++] = "-c"; - - if (argc < 2) - nargv[nargc++] = argv[0]; - else { - struct strbuf arg0 = STRBUF_INIT; - strbuf_addf(&arg0, "%s \"$@\"", argv[0]); - nargv[nargc++] = strbuf_detach(&arg0, NULL); + if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) { + nargv[nargc++] = "sh"; + nargv[nargc++] = "-c"; + + if (argc < 2) + nargv[nargc++] = argv[0]; + else { + struct strbuf arg0 = STRBUF_INIT; + strbuf_addf(&arg0, "%s \"$@\"", argv[0]); + nargv[nargc++] = strbuf_detach(&arg0, NULL); + } } for (argc = 0; argv[argc]; argc++) -- cgit v1.2.3 From bac8037081735a49bccdc3b3b1457129f9bcc451 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 30 Dec 2009 05:56:16 -0500 Subject: editor: use run_command's shell feature Now that run_command implements the same code in a more general form, we can make use of it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- editor.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/editor.c b/editor.c index 615f5754d6..d8340031d2 100644 --- a/editor.c +++ b/editor.c @@ -36,26 +36,9 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en return error("Terminal is dumb, but EDITOR unset"); if (strcmp(editor, ":")) { - size_t len = strlen(editor); - int i = 0; - int failed; - const char *args[6]; - struct strbuf arg0 = STRBUF_INIT; + const char *args[] = { editor, path, NULL }; - if (strcspn(editor, "|&;<>()$`\\\"' \t\n*?[#~=%") != len) { - /* there are specials */ - strbuf_addf(&arg0, "%s \"$@\"", editor); - args[i++] = "sh"; - args[i++] = "-c"; - args[i++] = arg0.buf; - } - args[i++] = editor; - args[i++] = path; - args[i] = NULL; - - failed = run_command_v_opt_cd_env(args, 0, NULL, env); - strbuf_release(&arg0); - if (failed) + if (run_command_v_opt_cd_env(args, RUN_USING_SHELL, NULL, env)) return error("There was a problem with the editor '%s'.", editor); } -- cgit v1.2.3 From 41a457e4f814a0e514548b3178e7692129f0fcfe Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 30 Dec 2009 06:01:09 -0500 Subject: textconv: use shell to run helper Currently textconv helpers are run directly. Running through the shell is useful because the user can provide a program with command line arguments, like "antiword -f". It also makes textconv more consistent with other parts of git, most of which run their helpers using the shell. The downside is that textconv helpers with shell metacharacters (like space) in the filename will be broken. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diff.c | 1 + t/t4030-diff-textconv.sh | 2 +- t/t4031-diff-rewrite-binary.sh | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/diff.c b/diff.c index 08bbd3e907..4af7c3f925 100644 --- a/diff.c +++ b/diff.c @@ -3771,6 +3771,7 @@ static char *run_textconv(const char *pgm, struct diff_filespec *spec, *arg = NULL; memset(&child, 0, sizeof(child)); + child.use_shell = 1; child.argv = argv; child.out = -1; if (start_command(&child) != 0 || diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh index a3f0897a52..c16d538f46 100755 --- a/t/t4030-diff-textconv.sh +++ b/t/t4030-diff-textconv.sh @@ -48,7 +48,7 @@ test_expect_success 'file is considered binary by plumbing' ' test_expect_success 'setup textconv filters' ' echo file diff=foo >.gitattributes && - git config diff.foo.textconv "$PWD"/hexdump && + git config diff.foo.textconv "\"$PWD\""/hexdump && git config diff.fail.textconv false ' diff --git a/t/t4031-diff-rewrite-binary.sh b/t/t4031-diff-rewrite-binary.sh index a894c60622..27fb31b401 100755 --- a/t/t4031-diff-rewrite-binary.sh +++ b/t/t4031-diff-rewrite-binary.sh @@ -54,7 +54,7 @@ chmod +x dump test_expect_success 'setup textconv' ' echo file diff=foo >.gitattributes && - git config diff.foo.textconv "$PWD"/dump + git config diff.foo.textconv "\"$PWD\""/dump ' test_expect_success 'rewrite diff respects textconv' ' -- cgit v1.2.3 From 7ed7fac45a696ba6343707c7c8f4b268f309be32 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 30 Dec 2009 06:03:35 -0500 Subject: diff: run external diff helper with shell This is mostly to make it more consistent with the rest of git, which uses the shell to exec helpers. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diff.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diff.c b/diff.c index 4af7c3f925..3b235e39ba 100644 --- a/diff.c +++ b/diff.c @@ -2275,7 +2275,7 @@ static void run_external_diff(const char *pgm, } *arg = NULL; fflush(NULL); - retval = run_command_v_opt(spawn_arg, 0); + retval = run_command_v_opt(spawn_arg, RUN_USING_SHELL); remove_tempfile(); if (retval) { fprintf(stderr, "external diff died, stopping at %s.\n", name); -- cgit v1.2.3 From 6396258368b76d432090c81096755dc1fbc89551 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 1 Jan 2010 23:15:18 +0100 Subject: t4030, t4031: work around bogus MSYS bash path conversion Recall that MSYS bash converts POSIX style absolute paths to Windows style absolute paths. Unfortunately, it converts a program argument that begins with a double-quote and otherwise looks like an absolute POSIX path, but in doing so, it strips everything past the second double-quote[*]. This case is triggered in the two test scripts. The work-around is to place the Windows style path returned by $(pwd) between the quotes to avoid the path conversion. [*] It is already bogus that a conversion is even considered when a program argument begins with a double-quote because it cannot be an absolute POSIX path. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t4030-diff-textconv.sh | 2 +- t/t4031-diff-rewrite-binary.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh index c16d538f46..88c5619ae7 100755 --- a/t/t4030-diff-textconv.sh +++ b/t/t4030-diff-textconv.sh @@ -48,7 +48,7 @@ test_expect_success 'file is considered binary by plumbing' ' test_expect_success 'setup textconv filters' ' echo file diff=foo >.gitattributes && - git config diff.foo.textconv "\"$PWD\""/hexdump && + git config diff.foo.textconv "\"$(pwd)\""/hexdump && git config diff.fail.textconv false ' diff --git a/t/t4031-diff-rewrite-binary.sh b/t/t4031-diff-rewrite-binary.sh index 27fb31b401..7e7b307a24 100755 --- a/t/t4031-diff-rewrite-binary.sh +++ b/t/t4031-diff-rewrite-binary.sh @@ -54,7 +54,7 @@ chmod +x dump test_expect_success 'setup textconv' ' echo file diff=foo >.gitattributes && - git config diff.foo.textconv "\"$PWD\""/dump + git config diff.foo.textconv "\"$(pwd)\""/dump ' test_expect_success 'rewrite diff respects textconv' ' -- cgit v1.2.3