Merge branch 'rs/run-command-env-array'

Add managed "env" array to child_process to clarify the lifetime
rules.

* rs/run-command-env-array:
use env_array member of struct child_process
run-command: add env_array, an optional argv_array for env

diff --git a/Documentation/technical/api-run-command.txt b/Documentation/technical/api-run-command.txt
index 842b8389eb867b6db654cf7fe29e8f10b92b259e..3f12fcdd4c788d73c204751404670d299c7f95d2 100644 (file)
--- a/Documentation/technical/api-run-command.txt
+++ b/Documentation/technical/api-run-command.txt
@@ -169,6 +169,11 @@ string pointers (NULL terminated) in .env:
 . If the string does not contain '=', it names an environment
   variable that will be removed from the child process's environment.
 
+If the .env member is NULL, `start_command` will point it at the
+.env_array `argv_array` (so you may use one or the other, but not both).
+The memory in .env_array will be cleaned up automatically during
+`finish_command` (or during `start_command` when it is unsuccessful).
+
 To specify a new initial working directory for the sub-process,
 specify it in the .dir member.
 

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 916315ff5c2f08bed1e52c8188939405c0b7a0e1..fc0393779c0dfd44cef7bdc3ec9a57e6d8892d52 100644 (file)
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -453,7 +453,6 @@ static const char *check_nonce(const char *buf, size_t len)
 static void prepare_push_cert_sha1(struct child_process *proc)
 {
        static int already_done;
-       struct argv_array env = ARGV_ARRAY_INIT;
 
        if (!push_cert.len)
                return;
@@ -487,20 +486,26 @@ static void prepare_push_cert_sha1(struct child_process *proc)
                nonce_status = check_nonce(push_cert.buf, bogs);
        }
        if (!is_null_sha1(push_cert_sha1)) {
-               argv_array_pushf(&env, "GIT_PUSH_CERT=%s", sha1_to_hex(push_cert_sha1));
-               argv_array_pushf(&env, "GIT_PUSH_CERT_SIGNER=%s",
+               argv_array_pushf(&proc->env_array, "GIT_PUSH_CERT=%s",
+                                sha1_to_hex(push_cert_sha1));
+               argv_array_pushf(&proc->env_array, "GIT_PUSH_CERT_SIGNER=%s",
                                 sigcheck.signer ? sigcheck.signer : "");
-               argv_array_pushf(&env, "GIT_PUSH_CERT_KEY=%s",
+               argv_array_pushf(&proc->env_array, "GIT_PUSH_CERT_KEY=%s",
                                 sigcheck.key ? sigcheck.key : "");
-               argv_array_pushf(&env, "GIT_PUSH_CERT_STATUS=%c", sigcheck.result);
+               argv_array_pushf(&proc->env_array, "GIT_PUSH_CERT_STATUS=%c",
+                                sigcheck.result);
                if (push_cert_nonce) {
-                       argv_array_pushf(&env, "GIT_PUSH_CERT_NONCE=%s", push_cert_nonce);
-                       argv_array_pushf(&env, "GIT_PUSH_CERT_NONCE_STATUS=%s", nonce_status);
+                       argv_array_pushf(&proc->env_array,
+                                        "GIT_PUSH_CERT_NONCE=%s",
+                                        push_cert_nonce);
+                       argv_array_pushf(&proc->env_array,
+                                        "GIT_PUSH_CERT_NONCE_STATUS=%s",
+                                        nonce_status);
                        if (nonce_status == NONCE_SLOP)
-                               argv_array_pushf(&env, "GIT_PUSH_CERT_NONCE_SLOP=%ld",
+                               argv_array_pushf(&proc->env_array,
+                                                "GIT_PUSH_CERT_NONCE_SLOP=%ld",
                                                 nonce_stamp_slop);
                }
-               proc->env = env.argv;
        }
 }
 

diff --git a/http-backend.c b/http-backend.c
index 9977c5d499e0397bf009c5e9954fe670ea7b034c..b6c0484fb24de853ac205233e4e07d2e4e94ed75 100644 (file)
--- a/http-backend.c
+++ b/http-backend.c
@@ -314,7 +314,6 @@ static void run_service(const char **argv)
        const char *encoding = getenv("HTTP_CONTENT_ENCODING");
        const char *user = getenv("REMOTE_USER");
        const char *host = getenv("REMOTE_ADDR");
-       struct argv_array env = ARGV_ARRAY_INIT;
        int gzipped_request = 0;
        struct child_process cld = CHILD_PROCESS_INIT;
 
@@ -329,13 +328,12 @@ static void run_service(const char **argv)
                host = "(none)";
 
        if (!getenv("GIT_COMMITTER_NAME"))
-               argv_array_pushf(&env, "GIT_COMMITTER_NAME=%s", user);
+               argv_array_pushf(&cld.env_array, "GIT_COMMITTER_NAME=%s", user);
        if (!getenv("GIT_COMMITTER_EMAIL"))
-               argv_array_pushf(&env, "GIT_COMMITTER_EMAIL=%s@http.%s",
-                                user, host);
+               argv_array_pushf(&cld.env_array,
+                                "GIT_COMMITTER_EMAIL=%s@http.%s", user, host);
 
        cld.argv = argv;
-       cld.env = env.argv;
        if (gzipped_request)
                cld.in = -1;
        cld.git_cmd = 1;
@@ -350,7 +348,6 @@ static void run_service(const char **argv)
 
        if (finish_command(&cld))
                exit(1);
-       argv_array_clear(&env);
 }
 
 static int show_text_ref(const char *name, const unsigned char *sha1,

diff --git a/pager.c b/pager.c
index b2b805af98552061a723f3b8e2ac3093d0b897f7..f6e8c331924496ca6656cd05d7de6497310502ab 100644 (file)
--- a/pager.c
+++ b/pager.c
@@ -74,17 +74,10 @@ void setup_pager(void)
        pager_process.use_shell = 1;
        pager_process.argv = pager_argv;
        pager_process.in = -1;
-       if (!getenv("LESS") || !getenv("LV")) {
-               static const char *env[3];
-               int i = 0;
-
-               if (!getenv("LESS"))
-                       env[i++] = "LESS=FRX";
-               if (!getenv("LV"))
-                       env[i++] = "LV=-c";
-               env[i] = NULL;
-               pager_process.env = env;
-       }
+       if (!getenv("LESS"))
+               argv_array_push(&pager_process.env_array, "LESS=FRX");
+       if (!getenv("LV"))
+               argv_array_push(&pager_process.env_array, "LV=-c");
        if (start_command(&pager_process))
                return;
 

diff --git a/run-command.c b/run-command.c
index 761f0fde40c91a3cc910c092c5aa9ad56f2f0042..46be513c480abf200148f7dcfc08c1c021410f7c 100644 (file)
--- a/run-command.c
+++ b/run-command.c
@@ -12,6 +12,7 @@ void child_process_init(struct child_process *child)
 {
        memset(child, 0, sizeof(*child));
        argv_array_init(&child->args);
+       argv_array_init(&child->env_array);
 }
 
 struct child_to_clean {
@@ -287,6 +288,8 @@ int start_command(struct child_process *cmd)
 
        if (!cmd->argv)
                cmd->argv = cmd->args.argv;
+       if (!cmd->env)
+               cmd->env = cmd->env_array.argv;
 
        /*
         * In case of errors we must keep the promise to close FDs
@@ -338,6 +341,7 @@ int start_command(struct child_process *cmd)
                        error("cannot create %s pipe for %s: %s",
                                str, cmd->argv[0], strerror(failed_errno));
                        argv_array_clear(&cmd->args);
+                       argv_array_clear(&cmd->env_array);
                        errno = failed_errno;
                        return -1;
                }
@@ -524,6 +528,7 @@ int start_command(struct child_process *cmd)
                else if (cmd->err)
                        close(cmd->err);
                argv_array_clear(&cmd->args);
+               argv_array_clear(&cmd->env_array);
                errno = failed_errno;
                return -1;
        }
@@ -550,6 +555,7 @@ int finish_command(struct child_process *cmd)
 {
        int ret = wait_or_whine(cmd->pid, cmd->argv[0]);
        argv_array_clear(&cmd->args);
+       argv_array_clear(&cmd->env_array);
        return ret;
 }
 

diff --git a/run-command.h b/run-command.h
index 1b135d1c960aa58e2fa4ad44ecc195835daac69a..2137315ee46f672d945e0d3e011a8cbfbd5582a7 100644 (file)
--- a/run-command.h
+++ b/run-command.h
@@ -10,6 +10,7 @@
 struct child_process {
        const char **argv;
        struct argv_array args;
+       struct argv_array env_array;
        pid_t pid;
        /*
         * Using .in, .out, .err:
@@ -44,7 +45,7 @@ struct child_process {
        unsigned clean_on_exit:1;
 };
 
-#define CHILD_PROCESS_INIT { NULL, ARGV_ARRAY_INIT }
+#define CHILD_PROCESS_INIT { NULL, ARGV_ARRAY_INIT, ARGV_ARRAY_INIT }
 void child_process_init(struct child_process *);
 
 int start_command(struct child_process *);

diff --git a/transport-helper.c b/transport-helper.c
index 9b2962041b573b6aa7880b87ce57fd8ddbd4cb11..6cd9dd1f9fe98e4c65156fba41172ba22d5be6de 100644 (file)
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -108,12 +108,6 @@ static struct child_process *get_helper(struct transport *transport)
        int refspec_alloc = 0;
        int duped;
        int code;
-       char git_dir_buf[sizeof(GIT_DIR_ENVIRONMENT) + PATH_MAX + 1];
-       const char *helper_env[] = {
-               git_dir_buf,
-               NULL
-       };
-
 
        if (data->helper)
                return data->helper;
@@ -129,8 +123,8 @@ static struct child_process *get_helper(struct transport *transport)
        helper->git_cmd = 0;
        helper->silent_exec_failure = 1;
 
-       snprintf(git_dir_buf, sizeof(git_dir_buf), "%s=%s", GIT_DIR_ENVIRONMENT, get_git_dir());
-       helper->env = helper_env;
+       argv_array_pushf(&helper->env_array, "%s=%s", GIT_DIR_ENVIRONMENT,
+                        get_git_dir());
 
        code = start_command(helper);
        if (code < 0 && errno == ENOENT)

diff --git a/wt-status.c b/wt-status.c
index f367066f9214791908e761d1120d1a6d809015fa..cdbc8d798aaff6fff833224ff94b83af49289a5a 100644 (file)
--- a/wt-status.c
+++ b/wt-status.c
@@ -726,14 +726,14 @@ static void wt_status_print_changed(struct wt_status *s)
 static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitted)
 {
        struct child_process sm_summary = CHILD_PROCESS_INIT;
-       struct argv_array env = ARGV_ARRAY_INIT;
        struct argv_array argv = ARGV_ARRAY_INIT;
        struct strbuf cmd_stdout = STRBUF_INIT;
        struct strbuf summary = STRBUF_INIT;
        char *summary_content;
        size_t len;
 
-       argv_array_pushf(&env, "GIT_INDEX_FILE=%s", s->index_file);
+       argv_array_pushf(&sm_summary.env_array, "GIT_INDEX_FILE=%s",
+                        s->index_file);
 
        argv_array_push(&argv, "submodule");
        argv_array_push(&argv, "summary");
@@ -745,14 +745,12 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
                argv_array_push(&argv, s->amend ? "HEAD^" : "HEAD");
 
        sm_summary.argv = argv.argv;
-       sm_summary.env = env.argv;
        sm_summary.git_cmd = 1;
        sm_summary.no_stdin = 1;
        fflush(s->fp);
        sm_summary.out = -1;
 
        run_command(&sm_summary);
-       argv_array_clear(&env);
        argv_array_clear(&argv);
 
        len = strbuf_read(&cmd_stdout, sm_summary.out, 1024);