Replace the convenience functions run_command_v_opt() et. al. and use
struct child_process and run_command() directly instead, for an overall
code reduction and a simpler and more flexible API that allows creating
argument lists without magic numbers and reduced risk of memory leaks.
Changes since v1:
- Do the return value fix earlier; it was only an afterthought before.
Keep the colon (no "while at it, ...").
- Break out the xstrdup(), oid_to_hex_r() and C99 cleanups.
- Convert tricky string arrays before strvecs because Ævar didn't like
the opposite order.
- Extend the example code in tmp-objdir.h so it still only requires
"cmd".
Ævar Arnfjörð Bjarmason (1):
merge: remove always-the-same "verbose" arguments
René Scharfe (11):
run-command: fix return value comment
am: simplify building "show" argument list
bisect: simplify building "checkout" argument list
bisect--helper: factor out do_bisect_run()
sequencer: simplify building argument list in do_exec()
use child_process member "args" instead of string array variable
use child_process members "args" and "env" directly
replace and remove run_command_v_opt_cd_env()
replace and remove run_command_v_opt_tr2()
replace and remove run_command_v_opt_cd_env_tr2()
replace and remove run_command_v_opt()
add-interactive.c | 9 ++-
bisect.c | 12 ++--
builtin/add.c | 19 +++--
builtin/am.c | 12 ++--
builtin/bisect--helper.c | 68 +++++++++---------
builtin/clone.c | 41 ++++++-----
builtin/difftool.c | 24 ++++---
builtin/fetch.c | 9 ++-
builtin/gc.c | 55 ++++++++++-----
builtin/merge-index.c | 4 +-
builtin/merge.c | 53 ++++++--------
builtin/pull.c | 147 +++++++++++++++++++--------------------
builtin/remote.c | 40 +++++------
compat/mingw.c | 11 +--
diff.c | 27 ++++---
fsmonitor-ipc.c | 10 ++-
git.c | 15 ++--
ll-merge.c | 7 +-
merge.c | 18 ++---
run-command.c | 35 ----------
run-command.h | 34 +--------
scalar.c | 13 ++--
sequencer.c | 32 ++++-----
shell.c | 17 +++--
t/helper/test-fake-ssh.c | 7 +-
t/helper/test-trace2.c | 4 +-
tmp-objdir.h | 6 +-
27 files changed, 346 insertions(+), 383 deletions(-)
Range-Diff gegen v1:
1: c0b2b88500 = 1: 113a9e0a81 merge: remove always-the-same "verbose" arguments
-: ---------- > 2: d10e70460b run-command: fix return value comment
-: ---------- > 3: c8cd913e39 am: simplify building "show" argument list
-: ---------- > 4: 4bb142e4a9 bisect: simplify building "checkout" argument list
2: 387db545d1 = 5: 5fcbe94eb4 bisect--helper: factor out do_bisect_run()
-: ---------- > 6: 962403cf22 sequencer: simplify building argument list in do_exec()
4: 348bc6d32c ! 7: f1689abe85 use child_process member "args" instead of string array variable
@@ Commit message
Use run_command() with a struct child_process variable and populate its
"args" member directly instead of building a string array and passing it
- to run_command_v_opt(). This avoids the use of magic index numbers.
-
- ## bisect.c ##
-@@ bisect.c: static struct oid_array skipped_revs;
-
- static struct object_id *current_bad_oid;
-
--static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
--
- static const char *term_bad;
- static const char *term_good;
-
-@@ bisect.c: static int is_expected_rev(const struct object_id *oid)
- enum bisect_error bisect_checkout(const struct object_id *bisect_rev,
- int no_checkout)
- {
-- char bisect_rev_hex[GIT_MAX_HEXSZ + 1];
- struct commit *commit;
- struct pretty_print_context pp = {0};
- struct strbuf commit_msg = STRBUF_INIT;
-
-- oid_to_hex_r(bisect_rev_hex, bisect_rev);
- update_ref(NULL, "BISECT_EXPECTED_REV", bisect_rev, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
-
-- argv_checkout[2] = bisect_rev_hex;
- if (no_checkout) {
- update_ref(NULL, "BISECT_HEAD", bisect_rev, NULL, 0,
- UPDATE_REFS_DIE_ON_ERR);
- } else {
-- if (run_command_v_opt(argv_checkout, RUN_GIT_CMD))
-+ struct child_process cmd = CHILD_PROCESS_INIT;
-+
-+ cmd.git_cmd = 1;
-+ strvec_pushl(&cmd.args, "checkout", "-q",
-+ oid_to_hex(bisect_rev), "--", NULL);
-+ if (run_command(&cmd))
- /*
- * Errors in `run_command()` itself, signaled by res < 0,
- * and errors in the child process, signaled by res > 0
-
- ## builtin/am.c ##
-@@ builtin/am.c: static int show_patch(struct am_state *state, enum show_patch_type sub_mode)
- int len;
-
- if (!is_null_oid(&state->orig_commit)) {
-- const char *av[4] = { "show", NULL, "--", NULL };
-- char *new_oid_str;
-- int ret;
-+ struct child_process cmd = CHILD_PROCESS_INIT;
-
-- av[1] = new_oid_str = xstrdup(oid_to_hex(&state->orig_commit));
-- ret = run_command_v_opt(av, RUN_GIT_CMD);
-- free(new_oid_str);
-- return ret;
-+ strvec_pushl(&cmd.args, "show", oid_to_hex(&state->orig_commit),
-+ "--", NULL);
-+ cmd.git_cmd = 1;
-+ return run_command(&cmd);
- }
-
- switch (sub_mode) {
+ to run_command_v_opt(). This avoids the use of magic index numbers and
+ makes simplifies the possible addition of more arguments in the future.
## builtin/difftool.c ##
@@ builtin/difftool.c: static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
@@ ll-merge.c: static enum ll_merge_result ll_ext_merge(const struct ll_merge_drive
if (fd < 0)
goto bad;
- ## sequencer.c ##
-@@ sequencer.c: static int error_failed_squash(struct repository *r,
-
- static int do_exec(struct repository *r, const char *command_line)
- {
-- const char *child_argv[] = { NULL, NULL };
-+ struct child_process cmd = CHILD_PROCESS_INIT;
- int dirty, status;
-
- fprintf(stderr, _("Executing: %s\n"), command_line);
-- child_argv[0] = command_line;
-- status = run_command_v_opt(child_argv, RUN_USING_SHELL);
-+ cmd.use_shell = 1;
-+ strvec_push(&cmd.args, command_line);
-+ status = run_command(&cmd);
-
- /* force re-reading of the cache */
- if (discard_index(r->index) < 0 || repo_read_index(r) < 0)
-
## t/helper/test-fake-ssh.c ##
@@ t/helper/test-fake-ssh.c: int cmd_main(int argc, const char **argv)
struct strbuf buf = STRBUF_INIT;
3: 7745e16df4 = 8: a467a4ecb5 use child_process members "args" and "env" directly
5: eeaa6aa86d ! 9: dc27358775 replace and remove run_command_v_opt_cd_env()
@@ run-command.h
@@ run-command.h: struct child_process {
/**
- * The functions: child_process_init, start_command, finish_command,
-- * run_command, run_command_v_opt, run_command_v_opt_cd_env, child_process_clear
-- * do the following:
-+ * run_command, run_command_v_opt, child_process_clear do the following:
+ * The functions: start_command, finish_command, run_command,
+- * run_command_v_opt, run_command_v_opt_cd_env do the following:
++ * run_command_v_opt do the following:
*
* - If a system call failed, errno is set and -1 is returned. A diagnostic
* is printed.
@@ run-command.h: int run_command_v_opt_tr2(const char **argv, int opt, const char
## tmp-objdir.h ##
@@
+ *
* Example:
*
++ * struct child_process child = CHILD_PROCESS_INIT;
* struct tmp_objdir *t = tmp_objdir_create("incoming");
- * if (!run_command_v_opt_cd_env(cmd, 0, NULL, tmp_objdir_env(t)) &&
- * !tmp_objdir_migrate(t))
-+ * strvec_pushv(&cmd.env, tmp_objdir_env(t));
-+ * if (!run_command(&cmd)) && !tmp_objdir_migrate(t))
++ * strvec_push(&child.args, cmd);
++ * strvec_pushv(&child.env, tmp_objdir_env(t));
++ * if (!run_command(&child)) && !tmp_objdir_migrate(t))
* printf("success!\n");
* else
* die("failed...tmp_objdir will clean up for us");
6: 95b5dcbb66 = 10: dcd65580c6 replace and remove run_command_v_opt_tr2()
7: 5e111ab053 ! 11: 389ec8ef54 replace and remove run_command_v_opt_cd_env_tr2()
@@ run-command.h: int run_auto_maintenance(int quiet);
/**
- * Convenience functions that encapsulate a sequence of
-+ * Convenience function that encapsulate a sequence of
++ * Convenience function that encapsulates a sequence of
* start_command() followed by finish_command(). The argument argv
* specifies the program and its arguments. The argument opt is zero
* or more of the flags `RUN_COMMAND_NO_STDIN`, `RUN_GIT_CMD`,
8: f1f438d724 ! 12: a6e7135e31 replace and remove run_command_v_opt()
@@ Commit message
Suggested-by: Jeff King <peff@xxxxxxxx>
+ ## bisect.c ##
+@@ bisect.c: enum bisect_error bisect_checkout(const struct object_id *bisect_rev,
+ update_ref(NULL, "BISECT_HEAD", bisect_rev, NULL, 0,
+ UPDATE_REFS_DIE_ON_ERR);
+ } else {
+- const char *argv_checkout[] = {
+- "checkout", "-q", oid_to_hex(bisect_rev), "--", NULL
+- };
++ struct child_process cmd = CHILD_PROCESS_INIT;
+
+- if (run_command_v_opt(argv_checkout, RUN_GIT_CMD))
++ cmd.git_cmd = 1;
++ strvec_pushl(&cmd.args, "checkout", "-q",
++ oid_to_hex(bisect_rev), "--", NULL);
++ if (run_command(&cmd))
+ /*
+ * Errors in `run_command()` itself, signaled by res < 0,
+ * and errors in the child process, signaled by res > 0
+
+ ## builtin/am.c ##
+@@ builtin/am.c: static int show_patch(struct am_state *state, enum show_patch_type sub_mode)
+ int len;
+
+ if (!is_null_oid(&state->orig_commit)) {
+- const char *av[] = {
+- "show", oid_to_hex(&state->orig_commit), "--", NULL
+- };
++ struct child_process cmd = CHILD_PROCESS_INIT;
+
+- return run_command_v_opt(av, RUN_GIT_CMD);
++ strvec_pushl(&cmd.args, "show", oid_to_hex(&state->orig_commit),
++ "--", NULL);
++ cmd.git_cmd = 1;
++ return run_command(&cmd);
+ }
+
+ switch (sub_mode) {
+
## builtin/bisect--helper.c ##
@@ builtin/bisect--helper.c: static enum bisect_error bisect_start(struct bisect_terms *terms, const char **a
strbuf_read_file(&start_head, git_path_bisect_start(), 0);
@@ run-command.c: int run_command(struct child_process *cmd)
## run-command.h ##
@@ run-command.h: struct child_process {
+ }
/**
- * The functions: child_process_init, start_command, finish_command,
-- * run_command, run_command_v_opt, child_process_clear do the following:
-+ * run_command, child_process_clear do the following:
+- * The functions: start_command, finish_command, run_command,
+- * run_command_v_opt do the following:
++ * The functions: start_command, finish_command, run_command do the following:
*
* - If a system call failed, errno is set and -1 is returned. A diagnostic
* is printed.
@@ run-command.h: int run_command(struct child_process *);
-#define RUN_CLOSE_OBJECT_STORE (1<<7)
-
-/**
-- * Convenience function that encapsulate a sequence of
+- * Convenience function that encapsulates a sequence of
- * start_command() followed by finish_command(). The argument argv
- * specifies the program and its arguments. The argument opt is zero
- * or more of the flags `RUN_COMMAND_NO_STDIN`, `RUN_GIT_CMD`,
@@ run-command.h: int run_command(struct child_process *);
* Execute the given command, sending "in" to its stdin, and capturing its
* stdout and stderr in the "out" and "err" strbufs. Any of the three may
+ ## sequencer.c ##
+@@ sequencer.c: static int error_failed_squash(struct repository *r,
+
+ static int do_exec(struct repository *r, const char *command_line)
+ {
+- const char *child_argv[] = { command_line, NULL };
++ struct child_process cmd = CHILD_PROCESS_INIT;
+ int dirty, status;
+
+ fprintf(stderr, _("Executing: %s\n"), command_line);
+- status = run_command_v_opt(child_argv, RUN_USING_SHELL);
++ cmd.use_shell = 1;
++ strvec_push(&cmd.args, command_line);
++ status = run_command(&cmd);
+
+ /* force re-reading of the cache */
+ if (discard_index(r->index) < 0 || repo_read_index(r) < 0)
+
## shell.c ##
@@ shell.c: static void cd_to_homedir(void)
static void run_shell(void)
9: 59677c9598 < -: ---------- run-command: fix return value comment
--
2.38.1
