# [PATCH 0/3] Miscellanous cleanups around start_command/run_command callers 4 messages from 2009-06-08 to 2009-06-08. Participants: Johannes Sixt. Thread: https://gitlist.dev/t/19750 ## Johannes Sixt, 2009-06-08 20:34 Subject: [PATCH 0/3] Miscellanous cleanups around start_command/run_command callers Message-ID: URL: https://gitlist.dev/e/cover.1244492658.git.j6t%40kdbg.org ``` This is just to flush a queue of unrelated changes before they pile up. I found these while I reviewed start_command and run_command call sites. Johannes Sixt (3): Simplify some instances of run_command() by using run_command_v_opt(). diff.c: plug a memory leak in an error path Simplify some 'fprintf(stderr); return -1;' by using 'return error()' builtin-help.c | 16 ++++++---------- builtin-receive-pack.c | 10 +++------- diff.c | 1 + ll-merge.c | 11 ++--------- merge-index.c | 42 +++++++++++++----------------------------- 5 files changed, 25 insertions(+), 55 deletions(-) ``` ## Johannes Sixt, 2009-06-08 20:34 Subject: [PATCH 1/3] Simplify some instances of run_command() by using run_command_v_opt(). Message-ID: URL: https://gitlist.dev/e/e4b190906055eb86077ca3e7257c983befcfb511.1244492659.git.j6t%40kdbg.org In-Reply-To: ``` Signed-off-by: Johannes Sixt --- builtin-receive-pack.c | 10 +++------- ll-merge.c | 11 ++--------- merge-index.c | 42 +++++++++++++----------------------------- 3 files changed, 18 insertions(+), 45 deletions(-) diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c index 0b08da9..33d345d 100644 --- a/builtin-receive-pack.c +++ b/builtin-receive-pack.c @@ -192,7 +192,6 @@ static int run_receive_hook(const char *hook_name) static int run_update_hook(struct command *cmd) { static const char update_hook[] = "hooks/update"; - struct child_process proc; const char *argv[5]; if (access(update_hook, X_OK) < 0) @@ -204,12 +203,9 @@ static int run_update_hook(struct command *cmd) argv[3] = sha1_to_hex(cmd->new_sha1); argv[4] = NULL; - memset(&proc, 0, sizeof(proc)); - proc.argv = argv; - proc.no_stdin = 1; - proc.stdout_to_stderr = 1; - - return hook_status(run_command(&proc), update_hook); + return hook_status(run_command_v_opt(argv, RUN_COMMAND_NO_STDIN | + RUN_COMMAND_STDOUT_TO_STDERR), + update_hook); } static int is_ref_checked_out(const char *ref) diff --git a/ll-merge.c b/ll-merge.c index 81c02ad..31d6f0a 100644 --- a/ll-merge.c +++ b/ll-merge.c @@ -175,8 +175,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn, { "B", temp[2] }, { NULL } }; - struct child_process child; - const char *args[20]; + const char *args[] = { "sh", "-c", NULL, NULL }; int status, fd, i; struct stat st; @@ -191,14 +190,8 @@ static int ll_ext_merge(const struct ll_merge_driver *fn, strbuf_expand(&cmd, fn->cmdline, strbuf_expand_dict_cb, &dict); - memset(&child, 0, sizeof(child)); - child.argv = args; - args[0] = "sh"; - args[1] = "-c"; args[2] = cmd.buf; - args[3] = NULL; - - status = run_command(&child); + status = run_command_v_opt(args, 0); if (status < -ERR_RUN_COMMAND_FORK) ; /* failure in run-command */ else diff --git a/merge-index.c b/merge-index.c index aa9cf23..19ddd03 100644 --- a/merge-index.c +++ b/merge-index.c @@ -3,45 +3,20 @@ #include "exec_cmd.h" static const char *pgm; -static const char *arguments[9]; static int one_shot, quiet; static int err; -static void run_program(void) -{ - struct child_process child; - memset(&child, 0, sizeof(child)); - child.argv = arguments; - if (run_command(&child)) { - if (one_shot) { - err++; - } else { - if (!quiet) - die("merge program failed"); - exit(1); - } - } -} - static int merge_entry(int pos, const char *path) { int found; + const char *arguments[] = { pgm, "", "", "", path, "", "", "", NULL }; + char hexbuf[4][60]; + char ownbuf[4][60]; if (pos >= active_nr) die("git merge-index: %s not in the cache", path); - arguments[0] = pgm; - arguments[1] = ""; - arguments[2] = ""; - arguments[3] = ""; - arguments[4] = path; - arguments[5] = ""; - arguments[6] = ""; - arguments[7] = ""; - arguments[8] = NULL; found = 0; do { - static char hexbuf[4][60]; - static char ownbuf[4][60]; struct cache_entry *ce = active_cache[pos]; int stage = ce_stage(ce); @@ -55,7 +30,16 @@ static int merge_entry(int pos, const char *path) } while (++pos < active_nr); if (!found) die("git merge-index: %s not in the cache", path); - run_program(); + + if (run_command_v_opt(arguments, 0)) { + if (one_shot) + err++; + else { + if (!quiet) + die("merge program failed"); + exit(1); + } + } return found; } -- 1.6.3.17.g1665f ``` ## Johannes Sixt, 2009-06-08 20:34 Subject: [PATCH 2/3] diff.c: plug a memory leak in an error path Message-ID: <8f487369337bdb1c9884b8f6e56530d25de85472.1244492659.git.j6t@kdbg.org> URL: https://gitlist.dev/e/8f487369337bdb1c9884b8f6e56530d25de85472.1244492659.git.j6t%40kdbg.org In-Reply-To: ``` Signed-off-by: Johannes Sixt --- diff.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/diff.c b/diff.c index 4d0a5b9..43835d7 100644 --- a/diff.c +++ b/diff.c @@ -3596,6 +3596,7 @@ static char *run_textconv(const char *pgm, struct diff_filespec *spec, if (start_command(&child) != 0 || strbuf_read(&buf, child.out, 0) < 0 || finish_command(&child) != 0) { + strbuf_release(&buf); remove_tempfile(); error("error running textconv command '%s'", pgm); return NULL; -- 1.6.3.17.g1665f ``` ## Johannes Sixt, 2009-06-08 20:34 Subject: [PATCH 3/3] Simplify some 'fprintf(stderr); return -1;' by using 'return error()' Message-ID: <7a02f50b758bfc6b9437f20cecea2d4628a5966a.1244492659.git.j6t@kdbg.org> URL: https://gitlist.dev/e/7a02f50b758bfc6b9437f20cecea2d4628a5966a.1244492659.git.j6t%40kdbg.org In-Reply-To: <8f487369337bdb1c9884b8f6e56530d25de85472.1244492659.git.j6t@kdbg.org> ``` Signed-off-by: Johannes Sixt --- builtin-help.c | 16 ++++++---------- 1 files changed, 6 insertions(+), 10 deletions(-) diff --git a/builtin-help.c b/builtin-help.c index af565fb..6e53b23 100644 --- a/builtin-help.c +++ b/builtin-help.c @@ -80,10 +80,9 @@ static int check_emacsclient_version(void) ec_process.argv = argv_ec; ec_process.err = -1; ec_process.stdout_to_stderr = 1; - if (start_command(&ec_process)) { - fprintf(stderr, "Failed to start emacsclient.\n"); - return -1; - } + if (start_command(&ec_process)) + return error("Failed to start emacsclient."); + strbuf_read(&buffer, ec_process.err, 20); close(ec_process.err); @@ -94,20 +93,17 @@ static int check_emacsclient_version(void) finish_command(&ec_process); if (prefixcmp(buffer.buf, "emacsclient")) { - fprintf(stderr, "Failed to parse emacsclient version.\n"); strbuf_release(&buffer); - return -1; + return error("Failed to parse emacsclient version."); } strbuf_remove(&buffer, 0, strlen("emacsclient")); version = atoi(buffer.buf); if (version < 22) { - fprintf(stderr, - "emacsclient version '%d' too old (< 22).\n", - version); strbuf_release(&buffer); - return -1; + return error("emacsclient version '%d' too old (< 22).", + version); } strbuf_release(&buffer); -- 1.6.3.17.g1665f ```