From 19a583dc39e3e1110f746e837c5ce1953d6ebfa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 19 Oct 2014 13:13:55 +0200 Subject: run-command: add env_array, an optional argv_array for env Similar to args, add a struct argv_array member to struct child_process that simplifies specifying the environment for children. It is freed automatically by finish_command() or if start_command() encounters an error. Suggested-by: Jeff King Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- Documentation/technical/api-run-command.txt | 5 +++++ run-command.c | 6 ++++++ run-command.h | 3 ++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Documentation/technical/api-run-command.txt b/Documentation/technical/api-run-command.txt index 842b8389e..3f12fcdd4 100644 --- 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/run-command.c b/run-command.c index 761f0fde4..46be513c4 100644 --- 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 @@ fail_pipe: 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 @@ fail_pipe: 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 1b135d1c9..2137315ee 100644 --- 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 *); -- cgit v1.2.1 From a915459097b72da9cc058172a54029352b684b0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 19 Oct 2014 13:14:20 +0200 Subject: use env_array member of struct child_process Convert users of struct child_process to using the managed env_array for specifying environment variables instead of supplying an array on the stack or bringing their own argv_array. This shortens and simplifies the code and ensures automatically that the allocated memory is freed after use. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 23 ++++++++++++++--------- http-backend.c | 9 +++------ pager.c | 15 ++++----------- transport-helper.c | 10 ++-------- wt-status.c | 6 ++---- 5 files changed, 25 insertions(+), 38 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index f2f6c6735..7593861d7 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -453,7 +453,6 @@ leave: 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 404e68259..e3e0dda0b 100644 --- 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 b2b805af9..f6e8c3319 100644 --- 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/transport-helper.c b/transport-helper.c index 2b24d51a2..033a22861 100644 --- 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 1bf5d7254..66d267c67 100644 --- 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); -- cgit v1.2.1