@matheusbsbs / matheus-bagliano / commits / c460c0ecdc

run-command: store an optional argv_array

All child_process structs need to point to an argv. For flexibility, we do not mandate the use of a dynamic argv_array. However, because the child_process does not own the memory, this can make memory management with a separate argv_array difficult. For example, if a function calls start_command but not finish_command, the argv memory must persist. The code needs to arrange to clean up the argv_array separately after finish_command runs. As a result, some of our code in this situation just leaks the memory. To help such cases, this patch adds a built-in argv_array to the child_process, which gets cleaned up automatically (both in finish_command and when start_command fails). Callers may use it if they choose, but can continue to use the raw argv if they wish. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed May 15, 2014 at 04:33 UTC c460c0ecdca46be85f6d9c845f9df7ce0e45c3c2

3 files changed +18 -1

Documentation/technical/api-run-command.txt

@@ -109,6 +109,13 @@ terminated), of which .argv[0] is the program name to run (usually

109 without a path). If the command to run is a git command, set argv[0] to

110 the command name without the 'git-' prefix and set .git_cmd = 1.

111

112 +Note that the ownership of the memory pointed to by .argv stays with the

113 +caller, but it should survive until `finish_command` completes. If the

114 +.argv member is NULL, `start_command` will point it at the .args

115 +`argv_array` (so you may use one or the other, but you must use exactly

116 +one). The memory in .args will be cleaned up automatically during

117 +`finish_command` (or during `start_command` when it is unsuccessful).

118 +

119 The members .in, .out, .err are used to redirect stdin, stdout,

120 stderr as follows:

121

run-command.c

+8 -1

@@ -279,6 +279,9 @@ int start_command(struct child_process *cmd)

279 int failed_errno;

280 char *str;

281

282 + if (!cmd->argv)

283 + cmd->argv = cmd->args.argv;

284 +

285 /*

286 * In case of errors we must keep the promise to close FDs

287 * that have been passed in via ->in and ->out.

@@ -328,6 +331,7 @@ int start_command(struct child_process *cmd)

331 fail_pipe:

332 error("cannot create %s pipe for %s: %s",

333 str, cmd->argv[0], strerror(failed_errno));

334 + argv_array_clear(&cmd->args);

335 errno = failed_errno;

336 return -1;

337 }

@@ -519,6 +523,7 @@ fail_pipe:

523 close_pair(fderr);

524 else if (cmd->err)

525 close(cmd->err);

526 + argv_array_clear(&cmd->args);

527 errno = failed_errno;

528 return -1;

529 }

@@ -543,7 +548,9 @@ fail_pipe:

548

549 int finish_command(struct child_process *cmd)

550 {

546 - return wait_or_whine(cmd->pid, cmd->argv[0]);

551 + int ret = wait_or_whine(cmd->pid, cmd->argv[0]);

552 + argv_array_clear(&cmd->args);

553 + return ret;

554 }

555

556 int run_command(struct child_process *cmd)

run-command.h

@@ -5,8 +5,11 @@

5 #include <pthread.h>

6 #endif

8 +#include "argv-array.h"

9 +

10 struct child_process {

11 const char **argv;

12 + struct argv_array args;

13 pid_t pid;

14 /*

15 * Using .in, .out, .err: