[PATCH 18/18] daemon: use an argv_array to exec children

From: Jeff King <peff@peff.net>
Date: Mar 28, 2017, 19:48 UTC
Message-ID: <20170328194810.upseqefljp2v3a6n@sigill.intra.peff.net>
In-Reply-To: <20170328194255.vf7nfzzmmzxsbn36@sigill.intra.peff.net>

Our struct child_process already has its own argv_array. Let's use that to avoid having to format options into separate buffers.

Note that we'll need to declare the child process outside of the run_service_command() helper to do this. But that opens up a further simplification, which is that the helper can append to our argument list, saving each caller from specifying "." manually.

Signed-off-by: Jeff King <peff@peff.net>
---
Of all the patches in this series, this is the one where I was most
undecided on whether the result was more readable or not.

My ulterior motive, of course, was to get rid of the unchecked snprintf() into timebuf. But it could also just become an xsnprintf() with no further changes.

 daemon.c | 38 +++++++++++++++++---------------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/daemon.c b/daemon.c
index 473e6b6b6..f70d27b82 100644
--- a/daemon.c
+++ b/daemon.c
@@ -449,46 +449,42 @@ static void copy_to_log(int fd)
 	fclose(fp);
 }
 
-static int run_service_command(const char **argv)
+static int run_service_command(struct child_process *cld)
 {
-	struct child_process cld = CHILD_PROCESS_INIT;
-
-	cld.argv = argv;
-	cld.git_cmd = 1;
-	cld.err = -1;
-	if (start_command(&cld))
+	argv_array_push(&cld->args, ".");
+	cld->git_cmd = 1;
+	cld->err = -1;
+	if (start_command(cld))
 		return -1;
 
 	close(0);
 	close(1);
 
-	copy_to_log(cld.err);
+	copy_to_log(cld->err);
 
-	return finish_command(&cld);
+	return finish_command(cld);
 }
 
 static int upload_pack(void)
 {
-	/* Timeout as string */
-	char timeout_buf[64];
-	const char *argv[] = { "upload-pack", "--strict", NULL, ".", NULL };
-
-	argv[2] = timeout_buf;
-
-	snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout);
-	return run_service_command(argv);
+	struct child_process cld = CHILD_PROCESS_INIT;
+	argv_array_pushl(&cld.args, "upload-pack", "--strict", NULL);
+	argv_array_pushf(&cld.args, "--timeout=%u", timeout);
+	return run_service_command(&cld);
 }
 
 static int upload_archive(void)
 {
-	static const char *argv[] = { "upload-archive", ".", NULL };
-	return run_service_command(argv);
+	struct child_process cld = CHILD_PROCESS_INIT;
+	argv_array_push(&cld.args, "upload-archive");
+	return run_service_command(&cld);
 }
 
 static int receive_pack(void)
 {
-	static const char *argv[] = { "receive-pack", ".", NULL };
-	return run_service_command(argv);
+	struct child_process cld = CHILD_PROCESS_INIT;
+	argv_array_push(&cld.args, "receive-pack");
+	return run_service_command(&cld);
 }
 
 static struct daemon_service daemon_service[] = {

-- 
2.12.2.845.g55fcf8b10