Merge branch 'jk/argv-array-for-child-process'
authorJunio C Hamano <gitster@pobox.com>
Mon, 16 Jun 2014 17:06:10 +0000 (10:06 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 16 Jun 2014 17:06:10 +0000 (10:06 -0700)
* 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

Documentation/technical/api-argv-array.txt
Documentation/technical/api-run-command.txt
argv-array.c
argv-array.h
column.c
connect.c
run-command.c
run-command.h
transport-helper.c
index a6b7d83a8eb05086cb88a9168117aa54cf018551..1a797812fb426189b03a814498dd7019c96607b4 100644 (file)
@@ -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.
index 5d7d7f2d32f58f8682ca5efb2fd98fe7e74247dd..69510ae57afcedbe13334160c6a26c52c42e8d54 100644 (file)
@@ -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:
 
index 9e960d549c158a4c761942bb17824c1d5e075d49..256741d2262b237c56b6730bea9d52c9b39b7ee3 100644 (file)
@@ -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);
-       }
-}
index 85ba438ac1e877fa865e5b4d2301733735fa5d03..c65e6e825a96efca6111184e5c50d2406064631f 100644 (file)
@@ -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 */
index 8d1ce88d1493cf18436b03f843c6d90190d47bb2..1a468debb4abdda3cbb90fcc4972b3d40da6f2ed 100644 (file)
--- 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);
index a983d061a90f0b720afa1a9b50247da299164d79..94a66502464400d5dfc47a4151cc2fd01780a0a4 100644 (file)
--- 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;
 }
index 75abc478c6da75471e5fdbb55c74efdfcf3ad160..be07d4ad335ba6df4653239b7ccd6930c91c9879 100644 (file)
@@ -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 @@ int start_command(struct child_process *cmd)
                        close_pair(fderr);
                else if (cmd->err)
                        close(cmd->err);
+               argv_array_clear(&cmd->args);
                errno = failed_errno;
                return -1;
        }
@@ -543,7 +548,9 @@ int start_command(struct child_process *cmd)
 
 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)
index 3653bfa6e123ca8497571a2ca1c47200c1b544a2..ea73de309bc65c3d00bb34ad84824ba72d85cbfe 100644 (file)
@@ -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:
index b468e4f88e730f88a1d231a574585ace56e7066a..d9063d714591bad2e44a4e58fdffc3bb1708157d 100644 (file)
@@ -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