Thread (1 message) 1 message, 1 author, 2022-10-14

Re: [PATCH 01/10] run-command.c: refactor run_command_*_tr2() to internal helpers

From: Junio C Hamano <hidden>
Date: 2022-10-14 18:22:25

Ævar Arnfjörð Bjarmason  [off-list ref] 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

quoted hunk
@@ -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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help