{"thread":{"id":"19750","subject":"[PATCH 0/3] Miscellanous cleanups around start_command/run_command callers","startedAt":"2009-06-08T20:34:28Z","lastAt":"2009-06-08T20:34:31Z","messageCount":4,"participants":["Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"115833","messageId":"cover.1244492658.git.j6t@kdbg.org","threadId":"19750","inReplyTo":null,"subject":"[PATCH 0/3] Miscellanous cleanups around start_command/run_command callers","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-06-08T20:34:28Z","receivedAt":"2009-06-08T20:34:28Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"This is just to flush a queue of unrelated changes before they pile up.\nI found these while I reviewed start_command and run_command call sites.\n\nJohannes Sixt (3):\n  Simplify some instances of run_command() by using\n    run_command_v_opt().\n  diff.c: plug a memory leak in an error path\n  Simplify some 'fprintf(stderr); return -1;' by using 'return error()'\n\n builtin-help.c         |   16 ++++++----------\n builtin-receive-pack.c |   10 +++-------\n diff.c                 |    1 +\n ll-merge.c             |   11 ++---------\n merge-index.c          |   42 +++++++++++++-----------------------------\n 5 files changed, 25 insertions(+), 55 deletions(-)\n"},{"id":"115834","messageId":"e4b190906055eb86077ca3e7257c983befcfb511.1244492659.git.j6t@kdbg.org","threadId":"19750","inReplyTo":"cover.1244492658.git.j6t@kdbg.org","subject":"[PATCH 1/3] Simplify some instances of run_command() by using run_command_v_opt().","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-06-08T20:34:29Z","receivedAt":"2009-06-08T20:34:29Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Signed-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n builtin-receive-pack.c |   10 +++-------\n ll-merge.c             |   11 ++---------\n merge-index.c          |   42 +++++++++++++-----------------------------\n 3 files changed, 18 insertions(+), 45 deletions(-)\n\ndiff --git a/builtin-receive-pack.c b/builtin-receive-pack.c\nindex 0b08da9..33d345d 100644\n--- a/builtin-receive-pack.c\n+++ b/builtin-receive-pack.c\n@@ -192,7 +192,6 @@ static int run_receive_hook(const char *hook_name)\n static int run_update_hook(struct command *cmd)\n {\n \tstatic const char update_hook[] = \"hooks/update\";\n-\tstruct child_process proc;\n \tconst char *argv[5];\n \n \tif (access(update_hook, X_OK) < 0)\n@@ -204,12 +203,9 @@ static int run_update_hook(struct command *cmd)\n \targv[3] = sha1_to_hex(cmd->new_sha1);\n \targv[4] = NULL;\n \n-\tmemset(&proc, 0, sizeof(proc));\n-\tproc.argv = argv;\n-\tproc.no_stdin = 1;\n-\tproc.stdout_to_stderr = 1;\n-\n-\treturn hook_status(run_command(&proc), update_hook);\n+\treturn hook_status(run_command_v_opt(argv, RUN_COMMAND_NO_STDIN |\n+\t\t\t\t\tRUN_COMMAND_STDOUT_TO_STDERR),\n+\t\t\tupdate_hook);\n }\n \n static int is_ref_checked_out(const char *ref)\ndiff --git a/ll-merge.c b/ll-merge.c\nindex 81c02ad..31d6f0a 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -175,8 +175,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,\n \t\t{ \"B\", temp[2] },\n \t\t{ NULL }\n \t};\n-\tstruct child_process child;\n-\tconst char *args[20];\n+\tconst char *args[] = { \"sh\", \"-c\", NULL, NULL };\n \tint status, fd, i;\n \tstruct stat st;\n \n@@ -191,14 +190,8 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,\n \n \tstrbuf_expand(&cmd, fn->cmdline, strbuf_expand_dict_cb, &dict);\n \n-\tmemset(&child, 0, sizeof(child));\n-\tchild.argv = args;\n-\targs[0] = \"sh\";\n-\targs[1] = \"-c\";\n \targs[2] = cmd.buf;\n-\targs[3] = NULL;\n-\n-\tstatus = run_command(&child);\n+\tstatus = run_command_v_opt(args, 0);\n \tif (status < -ERR_RUN_COMMAND_FORK)\n \t\t; /* failure in run-command */\n \telse\ndiff --git a/merge-index.c b/merge-index.c\nindex aa9cf23..19ddd03 100644\n--- a/merge-index.c\n+++ b/merge-index.c\n@@ -3,45 +3,20 @@\n #include \"exec_cmd.h\"\n \n static const char *pgm;\n-static const char *arguments[9];\n static int one_shot, quiet;\n static int err;\n \n-static void run_program(void)\n-{\n-\tstruct child_process child;\n-\tmemset(&child, 0, sizeof(child));\n-\tchild.argv = arguments;\n-\tif (run_command(&child)) {\n-\t\tif (one_shot) {\n-\t\t\terr++;\n-\t\t} else {\n-\t\t\tif (!quiet)\n-\t\t\t\tdie(\"merge program failed\");\n-\t\t\texit(1);\n-\t\t}\n-\t}\n-}\n-\n static int merge_entry(int pos, const char *path)\n {\n \tint found;\n+\tconst char *arguments[] = { pgm, \"\", \"\", \"\", path, \"\", \"\", \"\", NULL };\n+\tchar hexbuf[4][60];\n+\tchar ownbuf[4][60];\n \n \tif (pos >= active_nr)\n \t\tdie(\"git merge-index: %s not in the cache\", path);\n-\targuments[0] = pgm;\n-\targuments[1] = \"\";\n-\targuments[2] = \"\";\n-\targuments[3] = \"\";\n-\targuments[4] = path;\n-\targuments[5] = \"\";\n-\targuments[6] = \"\";\n-\targuments[7] = \"\";\n-\targuments[8] = NULL;\n \tfound = 0;\n \tdo {\n-\t\tstatic char hexbuf[4][60];\n-\t\tstatic char ownbuf[4][60];\n \t\tstruct cache_entry *ce = active_cache[pos];\n \t\tint stage = ce_stage(ce);\n \n@@ -55,7 +30,16 @@ static int merge_entry(int pos, const char *path)\n \t} while (++pos < active_nr);\n \tif (!found)\n \t\tdie(\"git merge-index: %s not in the cache\", path);\n-\trun_program();\n+\n+\tif (run_command_v_opt(arguments, 0)) {\n+\t\tif (one_shot)\n+\t\t\terr++;\n+\t\telse {\n+\t\t\tif (!quiet)\n+\t\t\t\tdie(\"merge program failed\");\n+\t\t\texit(1);\n+\t\t}\n+\t}\n \treturn found;\n }\n \n-- \n1.6.3.17.g1665f\n"},{"id":"115836","messageId":"8f487369337bdb1c9884b8f6e56530d25de85472.1244492659.git.j6t@kdbg.org","threadId":"19750","inReplyTo":"e4b190906055eb86077ca3e7257c983befcfb511.1244492659.git.j6t@kdbg.org","subject":"[PATCH 2/3] diff.c: plug a memory leak in an error path","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-06-08T20:34:30Z","receivedAt":"2009-06-08T20:34:30Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Signed-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n diff.c |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 4d0a5b9..43835d7 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3596,6 +3596,7 @@ static char *run_textconv(const char *pgm, struct diff_filespec *spec,\n \tif (start_command(&child) != 0 ||\n \t    strbuf_read(&buf, child.out, 0) < 0 ||\n \t    finish_command(&child) != 0) {\n+\t\tstrbuf_release(&buf);\n \t\tremove_tempfile();\n \t\terror(\"error running textconv command '%s'\", pgm);\n \t\treturn NULL;\n-- \n1.6.3.17.g1665f\n"},{"id":"115835","messageId":"7a02f50b758bfc6b9437f20cecea2d4628a5966a.1244492659.git.j6t@kdbg.org","threadId":"19750","inReplyTo":"8f487369337bdb1c9884b8f6e56530d25de85472.1244492659.git.j6t@kdbg.org","subject":"[PATCH 3/3] Simplify some 'fprintf(stderr); return -1;' by using 'return error()'","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-06-08T20:34:31Z","receivedAt":"2009-06-08T20:34:31Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Signed-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n builtin-help.c |   16 ++++++----------\n 1 files changed, 6 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin-help.c b/builtin-help.c\nindex af565fb..6e53b23 100644\n--- a/builtin-help.c\n+++ b/builtin-help.c\n@@ -80,10 +80,9 @@ static int check_emacsclient_version(void)\n \tec_process.argv = argv_ec;\n \tec_process.err = -1;\n \tec_process.stdout_to_stderr = 1;\n-\tif (start_command(&ec_process)) {\n-\t\tfprintf(stderr, \"Failed to start emacsclient.\\n\");\n-\t\treturn -1;\n-\t}\n+\tif (start_command(&ec_process))\n+\t\treturn error(\"Failed to start emacsclient.\");\n+\n \tstrbuf_read(&buffer, ec_process.err, 20);\n \tclose(ec_process.err);\n \n@@ -94,20 +93,17 @@ static int check_emacsclient_version(void)\n \tfinish_command(&ec_process);\n \n \tif (prefixcmp(buffer.buf, \"emacsclient\")) {\n-\t\tfprintf(stderr, \"Failed to parse emacsclient version.\\n\");\n \t\tstrbuf_release(&buffer);\n-\t\treturn -1;\n+\t\treturn error(\"Failed to parse emacsclient version.\");\n \t}\n \n \tstrbuf_remove(&buffer, 0, strlen(\"emacsclient\"));\n \tversion = atoi(buffer.buf);\n \n \tif (version < 22) {\n-\t\tfprintf(stderr,\n-\t\t\t\"emacsclient version '%d' too old (< 22).\\n\",\n-\t\t\tversion);\n \t\tstrbuf_release(&buffer);\n-\t\treturn -1;\n+\t\treturn error(\"emacsclient version '%d' too old (< 22).\",\n+\t\t\tversion);\n \t}\n \n \tstrbuf_release(&buffer);\n-- \n1.6.3.17.g1665f\n"}]}