summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2014-06-16 17:06:10 (GMT)
committerJunio C Hamano <gitster@pobox.com>2014-06-16 17:06:10 (GMT)
commit5b3a58d459171f49ee8d486e4ac399eb2678605d (patch)
tree86a262d494e9c2d741ef210bd7f8e7aa5e0fdf7c
parent45dc2927168cd9575a8ea81e78f6f5c39cb653f4 (diff)
parentff857e4ee8680af3988aff3383b1158f396a6fb2 (diff)
downloadgit-5b3a58d459171f49ee8d486e4ac399eb2678605d.zip
git-5b3a58d459171f49ee8d486e4ac399eb2678605d.tar.gz
git-5b3a58d459171f49ee8d486e4ac399eb2678605d.tar.bz2
Merge branch 'jk/argv-array-for-child-process'
* jk/argv-array-for-child-process: argv-array: drop "detach" code get_importer: use run-command's internal argv_array get_exporter: use argv_array get_helper: use run-command's internal argv_array git_connect: use argv_array run_column_filter: use argv_array run-command: store an optional argv_array
-rw-r--r--Documentation/technical/api-argv-array.txt8
-rw-r--r--Documentation/technical/api-run-command.txt7
-rw-r--r--argv-array.c20
-rw-r--r--argv-array.h2
-rw-r--r--column.c43
-rw-r--r--connect.c28
-rw-r--r--run-command.c9
-rw-r--r--run-command.h3
-rw-r--r--transport-helper.c44
9 files changed, 57 insertions, 107 deletions
diff --git a/Documentation/technical/api-argv-array.txt b/Documentation/technical/api-argv-array.txt
index a6b7d83..1a79781 100644
--- a/Documentation/technical/api-argv-array.txt
+++ b/Documentation/technical/api-argv-array.txt
@@ -53,11 +53,3 @@ Functions
`argv_array_clear`::
Free all memory associated with the array and return it to the
initial, empty state.
-
-`argv_array_detach`::
- Detach the argv array from the `struct argv_array`, transferring
- ownership of the allocated array and strings.
-
-`argv_array_free_detached`::
- Free the memory allocated by a `struct argv_array` that was later
- detached and is now no longer needed.
diff --git a/Documentation/technical/api-run-command.txt b/Documentation/technical/api-run-command.txt
index 5d7d7f2..69510ae 100644
--- a/Documentation/technical/api-run-command.txt
+++ b/Documentation/technical/api-run-command.txt
@@ -109,6 +109,13 @@ terminated), of which .argv[0] is the program name to run (usually
without a path). If the command to run is a git command, set argv[0] to
the command name without the 'git-' prefix and set .git_cmd = 1.
+Note that the ownership of the memory pointed to by .argv stays with the
+caller, but it should survive until `finish_command` completes. If the
+.argv member is NULL, `start_command` will point it at the .args
+`argv_array` (so you may use one or the other, but you must use exactly
+one). The memory in .args will be cleaned up automatically during
+`finish_command` (or during `start_command` when it is unsuccessful).
+
The members .in, .out, .err are used to redirect stdin, stdout,
stderr as follows:
diff --git a/argv-array.c b/argv-array.c
index 9e960d5..256741d 100644
--- a/argv-array.c
+++ b/argv-array.c
@@ -68,23 +68,3 @@ void argv_array_clear(struct argv_array *array)
}
argv_array_init(array);
}
-
-const char **argv_array_detach(struct argv_array *array, int *argc)
-{
- const char **argv =
- array->argv == empty_argv || array->argc == 0 ? NULL : array->argv;
- if (argc)
- *argc = array->argc;
- argv_array_init(array);
- return argv;
-}
-
-void argv_array_free_detached(const char **argv)
-{
- if (argv) {
- int i;
- for (i = 0; argv[i]; i++)
- free((char **)argv[i]);
- free(argv);
- }
-}
diff --git a/argv-array.h b/argv-array.h
index 85ba438..c65e6e8 100644
--- a/argv-array.h
+++ b/argv-array.h
@@ -19,7 +19,5 @@ LAST_ARG_MUST_BE_NULL
void argv_array_pushl(struct argv_array *, ...);
void argv_array_pop(struct argv_array *);
void argv_array_clear(struct argv_array *);
-const char **argv_array_detach(struct argv_array *array, int *argc);
-void argv_array_free_detached(const char **argv);
#endif /* ARGV_ARRAY_H */
diff --git a/column.c b/column.c
index 8d1ce88..1a468de 100644
--- a/column.c
+++ b/column.c
@@ -370,46 +370,29 @@ static struct child_process column_process;
int run_column_filter(int colopts, const struct column_options *opts)
{
- const char *av[10];
- int ret, ac = 0;
- struct strbuf sb_colopt = STRBUF_INIT;
- struct strbuf sb_width = STRBUF_INIT;
- struct strbuf sb_padding = STRBUF_INIT;
+ struct argv_array *argv;
if (fd_out != -1)
return -1;
- av[ac++] = "column";
- strbuf_addf(&sb_colopt, "--raw-mode=%d", colopts);
- av[ac++] = sb_colopt.buf;
- if (opts && opts->width) {
- strbuf_addf(&sb_width, "--width=%d", opts->width);
- av[ac++] = sb_width.buf;
- }
- if (opts && opts->indent) {
- av[ac++] = "--indent";
- av[ac++] = opts->indent;
- }
- if (opts && opts->padding) {
- strbuf_addf(&sb_padding, "--padding=%d", opts->padding);
- av[ac++] = sb_padding.buf;
- }
- av[ac] = NULL;
+ memset(&column_process, 0, sizeof(column_process));
+ argv = &column_process.args;
+
+ argv_array_push(argv, "column");
+ argv_array_pushf(argv, "--raw-mode=%d", colopts);
+ if (opts && opts->width)
+ argv_array_pushf(argv, "--width=%d", opts->width);
+ if (opts && opts->indent)
+ argv_array_pushf(argv, "--indent=%s", opts->indent);
+ if (opts && opts->padding)
+ argv_array_pushf(argv, "--padding=%d", opts->padding);
fflush(stdout);
- memset(&column_process, 0, sizeof(column_process));
column_process.in = -1;
column_process.out = dup(1);
column_process.git_cmd = 1;
- column_process.argv = av;
-
- ret = start_command(&column_process);
-
- strbuf_release(&sb_colopt);
- strbuf_release(&sb_width);
- strbuf_release(&sb_padding);
- if (ret)
+ if (start_command(&column_process))
return -2;
fd_out = dup(1);
diff --git a/connect.c b/connect.c
index a983d06..94a6650 100644
--- a/connect.c
+++ b/connect.c
@@ -534,22 +534,18 @@ static int git_use_proxy(const char *host)
static struct child_process *git_proxy_connect(int fd[2], char *host)
{
const char *port = STR(DEFAULT_GIT_PORT);
- const char **argv;
struct child_process *proxy;
get_host_and_port(&host, &port);
- argv = xmalloc(sizeof(*argv) * 4);
- argv[0] = git_proxy_command;
- argv[1] = host;
- argv[2] = port;
- argv[3] = NULL;
proxy = xcalloc(1, sizeof(*proxy));
- proxy->argv = argv;
+ argv_array_push(&proxy->args, git_proxy_command);
+ argv_array_push(&proxy->args, host);
+ argv_array_push(&proxy->args, port);
proxy->in = -1;
proxy->out = -1;
if (start_command(proxy))
- die("cannot start proxy %s", argv[0]);
+ die("cannot start proxy %s", git_proxy_command);
fd[0] = proxy->out; /* read from proxy stdout */
fd[1] = proxy->in; /* write to proxy stdin */
return proxy;
@@ -663,7 +659,6 @@ struct child_process *git_connect(int fd[2], const char *url,
char *hostandport, *path;
struct child_process *conn = &no_fork;
enum protocol protocol;
- const char **arg;
struct strbuf cmd = STRBUF_INIT;
/* Without this we cannot rely on waitpid() to tell
@@ -707,7 +702,6 @@ struct child_process *git_connect(int fd[2], const char *url,
sq_quote_buf(&cmd, path);
conn->in = conn->out = -1;
- conn->argv = arg = xcalloc(7, sizeof(*arg));
if (protocol == PROTO_SSH) {
const char *ssh = getenv("GIT_SSH");
int putty = ssh && strcasestr(ssh, "plink");
@@ -718,22 +712,21 @@ struct child_process *git_connect(int fd[2], const char *url,
if (!ssh) ssh = "ssh";
- *arg++ = ssh;
+ argv_array_push(&conn->args, ssh);
if (putty && !strcasestr(ssh, "tortoiseplink"))
- *arg++ = "-batch";
+ argv_array_push(&conn->args, "-batch");
if (port) {
/* P is for PuTTY, p is for OpenSSH */
- *arg++ = putty ? "-P" : "-p";
- *arg++ = port;
+ argv_array_push(&conn->args, putty ? "-P" : "-p");
+ argv_array_push(&conn->args, port);
}
- *arg++ = ssh_host;
+ argv_array_push(&conn->args, ssh_host);
} else {
/* remove repo-local variables from the environment */
conn->env = local_repo_env;
conn->use_shell = 1;
}
- *arg++ = cmd.buf;
- *arg = NULL;
+ argv_array_push(&conn->args, cmd.buf);
if (start_command(conn))
die("unable to fork");
@@ -759,7 +752,6 @@ int finish_connect(struct child_process *conn)
return 0;
code = finish_command(conn);
- free(conn->argv);
free(conn);
return code;
}
diff --git a/run-command.c b/run-command.c
index 75abc47..be07d4a 100644
--- a/run-command.c
+++ b/run-command.c
@@ -279,6 +279,9 @@ int start_command(struct child_process *cmd)
int failed_errno;
char *str;
+ if (!cmd->argv)
+ cmd->argv = cmd->args.argv;
+
/*
* In case of errors we must keep the promise to close FDs
* that have been passed in via ->in and ->out.
@@ -328,6 +331,7 @@ int start_command(struct child_process *cmd)
fail_pipe:
error("cannot create %s pipe for %s: %s",
str, cmd->argv[0], strerror(failed_errno));
+ argv_array_clear(&cmd->args);
errno = failed_errno;
return -1;
}
@@ -519,6 +523,7 @@ fail_pipe:
close_pair(fderr);
else if (cmd->err)
close(cmd->err);
+ argv_array_clear(&cmd->args);
errno = failed_errno;
return -1;
}
@@ -543,7 +548,9 @@ fail_pipe:
int finish_command(struct child_process *cmd)
{
- return wait_or_whine(cmd->pid, cmd->argv[0]);
+ int ret = wait_or_whine(cmd->pid, cmd->argv[0]);
+ argv_array_clear(&cmd->args);
+ return ret;
}
int run_command(struct child_process *cmd)
diff --git a/run-command.h b/run-command.h
index 3653bfa..ea73de3 100644
--- a/run-command.h
+++ b/run-command.h
@@ -5,8 +5,11 @@
#include <pthread.h>
#endif
+#include "argv-array.h"
+
struct child_process {
const char **argv;
+ struct argv_array args;
pid_t pid;
/*
* Using .in, .out, .err:
diff --git a/transport-helper.c b/transport-helper.c
index b468e4f..d9063d7 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -101,7 +101,6 @@ static void do_take_over(struct transport *transport)
static struct child_process *get_helper(struct transport *transport)
{
struct helper_data *data = transport->data;
- struct argv_array argv = ARGV_ARRAY_INIT;
struct strbuf buf = STRBUF_INIT;
struct child_process *helper;
const char **refspecs = NULL;
@@ -123,10 +122,9 @@ static struct child_process *get_helper(struct transport *transport)
helper->in = -1;
helper->out = -1;
helper->err = 0;
- argv_array_pushf(&argv, "git-remote-%s", data->name);
- argv_array_push(&argv, transport->remote->name);
- argv_array_push(&argv, remove_ext_force(transport->url));
- helper->argv = argv_array_detach(&argv, NULL);
+ argv_array_pushf(&helper->args, "git-remote-%s", data->name);
+ argv_array_push(&helper->args, transport->remote->name);
+ argv_array_push(&helper->args, remove_ext_force(transport->url));
helper->git_cmd = 0;
helper->silent_exec_failure = 1;
@@ -245,7 +243,6 @@ static int disconnect_helper(struct transport *transport)
close(data->helper->out);
fclose(data->out);
res = finish_command(data->helper);
- argv_array_free_detached(data->helper->argv);
free(data->helper);
data->helper = NULL;
}
@@ -397,18 +394,16 @@ static int get_importer(struct transport *transport, struct child_process *fasti
{
struct child_process *helper = get_helper(transport);
struct helper_data *data = transport->data;
- struct argv_array argv = ARGV_ARRAY_INIT;
int cat_blob_fd, code;
memset(fastimport, 0, sizeof(*fastimport));
fastimport->in = helper->out;
- argv_array_push(&argv, "fast-import");
- argv_array_push(&argv, debug ? "--stats" : "--quiet");
+ argv_array_push(&fastimport->args, "fast-import");
+ argv_array_push(&fastimport->args, debug ? "--stats" : "--quiet");
if (data->bidi_import) {
cat_blob_fd = xdup(helper->in);
- argv_array_pushf(&argv, "--cat-blob-fd=%d", cat_blob_fd);
+ argv_array_pushf(&fastimport->args, "--cat-blob-fd=%d", cat_blob_fd);
}
- fastimport->argv = argv.argv;
fastimport->git_cmd = 1;
code = start_command(fastimport);
@@ -421,30 +416,24 @@ static int get_exporter(struct transport *transport,
{
struct helper_data *data = transport->data;
struct child_process *helper = get_helper(transport);
- int argc = 0, i;
- struct strbuf tmp = STRBUF_INIT;
+ int i;
memset(fastexport, 0, sizeof(*fastexport));
/* we need to duplicate helper->in because we want to use it after
* fastexport is done with it. */
fastexport->out = dup(helper->in);
- fastexport->argv = xcalloc(6 + revlist_args->nr, sizeof(*fastexport->argv));
- fastexport->argv[argc++] = "fast-export";
- fastexport->argv[argc++] = "--use-done-feature";
- fastexport->argv[argc++] = data->signed_tags ?
- "--signed-tags=verbatim" : "--signed-tags=warn-strip";
- if (data->export_marks) {
- strbuf_addf(&tmp, "--export-marks=%s.tmp", data->export_marks);
- fastexport->argv[argc++] = strbuf_detach(&tmp, NULL);
- }
- if (data->import_marks) {
- strbuf_addf(&tmp, "--import-marks=%s", data->import_marks);
- fastexport->argv[argc++] = strbuf_detach(&tmp, NULL);
- }
+ argv_array_push(&fastexport->args, "fast-export");
+ argv_array_push(&fastexport->args, "--use-done-feature");
+ argv_array_push(&fastexport->args, data->signed_tags ?
+ "--signed-tags=verbatim" : "--signed-tags=warn-strip");
+ if (data->export_marks)
+ argv_array_pushf(&fastexport->args, "--export-marks=%s.tmp", data->export_marks);
+ if (data->import_marks)
+ argv_array_pushf(&fastexport->args, "--import-marks=%s", data->import_marks);
for (i = 0; i < revlist_args->nr; i++)
- fastexport->argv[argc++] = revlist_args->items[i].string;
+ argv_array_push(&fastexport->args, revlist_args->items[i].string);
fastexport->git_cmd = 1;
return start_command(fastexport);
@@ -485,7 +474,6 @@ static int fetch_with_import(struct transport *transport,
if (finish_command(&fastimport))
die("Error while running fast-import");
- argv_array_free_detached(fastimport.argv);
/*
* The fast-import stream of a remote helper that advertises