| Message ID | patch-01.10-c1f701af6e8-20221014T153426Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | run-command API: add run_command_{l,sv}_opt() | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > +static void run_command_set_opts(struct child_process *cmd, int opt) > +{ > + cmd->no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0; > + 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; > + cmd->clean_on_exit = opt & RUN_CLEAN_ON_EXIT ? 1 : 0; > + cmd->wait_after_clean = opt & RUN_WAIT_AFTER_CLEAN ? 1 : 0; > + cmd->close_object_store = opt & RUN_CLOSE_OBJECT_STORE ? 1 : 0; > +} This, combined with the change in the caller that loses the corresponding lines, does make sense. But > @@ -1019,24 +1031,26 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const > return run_command_v_opt_cd_env_tr2(argv, opt, dir, env, NULL); > } > > +static int run_command_v_opt_cd_env_tr2_1(struct child_process *cmd, int opt, > + const char *dir, > + const char *const *env, > + const char *tr2_class) > +{ > + run_command_set_opts(cmd, opt); > + cmd->dir = dir; > + if (env) > + strvec_pushv(&cmd->env, (const char **)env); > + cmd->trace2_child_class = tr2_class; > + return run_command(cmd); > +} This? The original caller took argv and copied it into cmd.args itself and the updated caller still does so because this new helper doesn't do so for it, but it no longer does the copying of env because this helper does it. Unless we will add several more callers of this helper in later steps, this sounds a bit too specialized to the way the first single caller used to do it. But let's keep reading to see if it is worth adding it. > int run_command_v_opt_cd_env_tr2(const char **argv, int opt, const char *dir, > const char *const *env, const char *tr2_class) > { > struct child_process cmd = CHILD_PROCESS_INIT; > + > strvec_pushv(&cmd.args, argv); > - cmd.no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0; > - 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; > - cmd.clean_on_exit = opt & RUN_CLEAN_ON_EXIT ? 1 : 0; > - cmd.wait_after_clean = opt & RUN_WAIT_AFTER_CLEAN ? 1 : 0; > - cmd.close_object_store = opt & RUN_CLOSE_OBJECT_STORE ? 1 : 0; > - cmd.dir = dir; > - if (env) > - strvec_pushv(&cmd.env, (const char **)env); > - cmd.trace2_child_class = tr2_class; > - return run_command(&cmd); > + return run_command_v_opt_cd_env_tr2_1(&cmd, opt, dir, env, tr2_class); > } > > #ifndef NO_PTHREADS
diff --git a/run-command.c b/run-command.c index 5ec3a46dccf..cf4a431c839 100644 --- a/run-command.c +++ b/run-command.c @@ -1004,6 +1004,18 @@ int run_command(struct child_process *cmd) return finish_command(cmd); } +static void run_command_set_opts(struct child_process *cmd, int opt) +{ + cmd->no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0; + 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; + cmd->clean_on_exit = opt & RUN_CLEAN_ON_EXIT ? 1 : 0; + cmd->wait_after_clean = opt & RUN_WAIT_AFTER_CLEAN ? 1 : 0; + cmd->close_object_store = opt & RUN_CLOSE_OBJECT_STORE ? 1 : 0; +} + int run_command_v_opt(const char **argv, int opt) { return run_command_v_opt_cd_env(argv, opt, NULL, NULL); @@ -1019,24 +1031,26 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const return run_command_v_opt_cd_env_tr2(argv, opt, dir, env, NULL); } +static int run_command_v_opt_cd_env_tr2_1(struct child_process *cmd, int opt, + const char *dir, + const char *const *env, + const char *tr2_class) +{ + run_command_set_opts(cmd, opt); + cmd->dir = dir; + if (env) + strvec_pushv(&cmd->env, (const char **)env); + cmd->trace2_child_class = tr2_class; + return run_command(cmd); +} + int run_command_v_opt_cd_env_tr2(const char **argv, int opt, const char *dir, const char *const *env, const char *tr2_class) { struct child_process cmd = CHILD_PROCESS_INIT; + strvec_pushv(&cmd.args, argv); - cmd.no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0; - 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; - cmd.clean_on_exit = opt & RUN_CLEAN_ON_EXIT ? 1 : 0; - cmd.wait_after_clean = opt & RUN_WAIT_AFTER_CLEAN ? 1 : 0; - cmd.close_object_store = opt & RUN_CLOSE_OBJECT_STORE ? 1 : 0; - cmd.dir = dir; - if (env) - strvec_pushv(&cmd.env, (const char **)env); - cmd.trace2_child_class = tr2_class; - return run_command(&cmd); + return run_command_v_opt_cd_env_tr2_1(&cmd, opt, dir, env, tr2_class); } #ifndef NO_PTHREADS
Refactor the run_command_v_opt_cd_env_tr2() function to helpers which takes a "struct child_process *cmd" argument. This will allow for adding a helper which won't need to copy a set of arguments to the "cmd.args" we'd otherwise have to populate from the "argv". Splitting out the part of the function that sets the various members from "opt" will help in the subsequent commit. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- run-command.c | 40 +++++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-)