From 3ddf0968c2f64a9f9fa6880d9c22d40459c88335 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 24 May 2011 18:49:36 -0400 Subject: config: make environment parsing routines static Nobody outside of git_config_from_parameters should need to use the GIT_CONFIG_PARAMETERS parsing functions, so let's make them private. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/cache.h b/cache.h index 123dd4b..45bb36d 100644 --- a/cache.h +++ b/cache.h @@ -982,8 +982,6 @@ typedef int (*config_fn_t)(const char *, const char *, void *); extern int git_default_config(const char *, const char *, void *); extern int git_config_from_file(config_fn_t fn, const char *, void *); extern void git_config_push_parameter(const char *text); -extern int git_config_parse_parameter(const char *text); -extern int git_config_parse_environment(void); extern int git_config_from_parameters(config_fn_t fn, void *data); extern int git_config(config_fn_t fn, void *); extern int git_config_early(config_fn_t fn, void *, const char *repo_config); diff --git a/config.c b/config.c index c431f41..230fe2c 100644 --- a/config.c +++ b/config.c @@ -48,7 +48,7 @@ void git_config_push_parameter(const char *text) strbuf_release(&env); } -int git_config_parse_parameter(const char *text) +static int git_config_parse_parameter(const char *text) { struct config_item *ct; struct strbuf tmp = STRBUF_INIT; @@ -75,7 +75,7 @@ int git_config_parse_parameter(const char *text) return 0; } -int git_config_parse_environment(void) { +static int git_config_parse_environment(void) { const char *env = getenv(CONFIG_DATA_ENVIRONMENT); char *envw; const char **argv = NULL; -- cgit v0.10.2-6-g49f6 From 5a0c9eeb89a19a05cbc2bf570f69f1724ef873dd Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 24 May 2011 18:49:45 -0400 Subject: git_config: don't peek at global config_parameters The config_parameters list in config.c is an implementation detail of git_config_from_parameters; instead, that function should tell us whether it found anything. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/config.c b/config.c index 230fe2c..065c5b7 100644 --- a/config.c +++ b/config.c @@ -832,7 +832,7 @@ int git_config_from_parameters(config_fn_t fn, void *data) for (ct = config_parameters; ct; ct = ct->next) if (fn(ct->name, ct->value, data) < 0) return -1; - return 0; + return config_parameters != NULL; } int git_config_early(config_fn_t fn, void *data, const char *repo_config) @@ -864,9 +864,16 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config) found += 1; } - ret += git_config_from_parameters(fn, data); - if (config_parameters) - found += 1; + switch (git_config_from_parameters(fn, data)) { + case -1: /* error */ + ret--; + break; + case 0: /* found nothing */ + break; + default: /* found at least one item */ + found++; + break; + } if (found == 0) return -1; -- cgit v0.10.2-6-g49f6 From 06eb708f331f0829081f4f3fb3c465eaae345deb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 24 May 2011 18:49:55 -0400 Subject: config: always parse GIT_CONFIG_PARAMETERS during git_config Previously we parsed GIT_CONFIG_PARAMETERS lazily into a linked list, and then checked that list during future invocations of git_config. However, that ignores the fact that the environment variable could change during our run (e.g., because we parse more "-c" as part of an alias). Instead, let's just re-parse the environment variable each time. It's generally not very big, and it's no more work than parsing the config files, anyway. As a bonus, we can ditch all of the linked list storage code entirely, making the code much simpler. The test unfortunately still does not pass because of an unrelated bug in handle_options. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/config.c b/config.c index 065c5b7..8220c0c 100644 --- a/config.c +++ b/config.c @@ -20,15 +20,6 @@ static int zlib_compression_seen; const char *config_exclusive_filename = NULL; -struct config_item -{ - struct config_item *next; - char *name; - char *value; -}; -static struct config_item *config_parameters; -static struct config_item **config_parameters_tail = &config_parameters; - static void lowercase(char *p) { for (; *p; p++) @@ -48,9 +39,9 @@ void git_config_push_parameter(const char *text) strbuf_release(&env); } -static int git_config_parse_parameter(const char *text) +static int git_config_parse_parameter(const char *text, + config_fn_t fn, void *data) { - struct config_item *ct; struct strbuf tmp = STRBUF_INIT; struct strbuf **pair; strbuf_addstr(&tmp, text); @@ -60,22 +51,19 @@ static int git_config_parse_parameter(const char *text) strbuf_trim(pair[0]); if (!pair[0]->len) { strbuf_list_free(pair); - return -1; + return error("bogus config parameter: %s", text); } - ct = xcalloc(1, sizeof(struct config_item)); - ct->name = strbuf_detach(pair[0], NULL); - if (pair[1]) { - strbuf_trim(pair[1]); - ct->value = strbuf_detach(pair[1], NULL); + lowercase(pair[0]->buf); + if (fn(pair[0]->buf, pair[1] ? pair[1]->buf : NULL, data) < 0) { + strbuf_list_free(pair); + return -1; } strbuf_list_free(pair); - lowercase(ct->name); - *config_parameters_tail = ct; - config_parameters_tail = &ct->next; return 0; } -static int git_config_parse_environment(void) { +int git_config_from_parameters(config_fn_t fn, void *data) +{ const char *env = getenv(CONFIG_DATA_ENVIRONMENT); char *envw; const char **argv = NULL; @@ -93,8 +81,7 @@ static int git_config_parse_environment(void) { } for (i = 0; i < nr; i++) { - if (git_config_parse_parameter(argv[i]) < 0) { - error("bogus config parameter: %s", argv[i]); + if (git_config_parse_parameter(argv[i], fn, data) < 0) { free(argv); free(envw); return -1; @@ -103,7 +90,7 @@ static int git_config_parse_environment(void) { free(argv); free(envw); - return 0; + return nr > 0; } static int get_next_char(void) @@ -819,22 +806,6 @@ int git_config_global(void) return !git_env_bool("GIT_CONFIG_NOGLOBAL", 0); } -int git_config_from_parameters(config_fn_t fn, void *data) -{ - static int loaded_environment; - const struct config_item *ct; - - if (!loaded_environment) { - if (git_config_parse_environment() < 0) - return -1; - loaded_environment = 1; - } - for (ct = config_parameters; ct; ct = ct->next) - if (fn(ct->name, ct->value, data) < 0) - return -1; - return config_parameters != NULL; -} - int git_config_early(config_fn_t fn, void *data, const char *repo_config) { int ret = 0, found = 0; diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index d0ab8ff..52c9ac9 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -854,4 +854,11 @@ test_expect_success 'git -c "key=value" support' ' test_must_fail git -c core.name=value config name ' +test_expect_failure 'git -c works with aliases of builtins' ' + git config alias.checkconfig "-c foo.check=bar config foo.check" && + echo bar >expect && + git checkconfig >actual && + test_cmp expect actual +' + test_done -- cgit v0.10.2-6-g49f6 From 73546c085d49694c5e54b421f80bde6bc25006fb Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 24 May 2011 18:50:35 -0400 Subject: handle_options(): do not miscount how many arguments were used The handle_options() function advances the base of the argument array and returns the number of arguments it used. The caller in handle_alias() wants to reallocate the argv array it passes to this function, and attempts to do so by subtracting the returned value to compensate for the change handle_options() makes to the new_argv. But handle_options() did not correctly count when "-c " is given, causing a wrong pointer to be passed to realloc(). Fix it by saving the original argv at the beginning of handle_options(), and return the difference between the final value of argv, which will relieve the places that move the array pointer from the additional burden of keeping track of "handled" counter. Noticed-by: Kazuki Tsujimoto Signed-off-by: Junio C Hamano Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/git.c b/git.c index 6793178..6bea8ee 100644 --- a/git.c +++ b/git.c @@ -55,7 +55,7 @@ static void commit_pager_choice(void) { static int handle_options(const char ***argv, int *argc, int *envchanged) { - int handled = 0; + const char **orig_argv = *argv; while (*argc > 0) { const char *cmd = (*argv)[0]; @@ -105,7 +105,6 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; (*argv)++; (*argc)--; - handled++; } else if (!prefixcmp(cmd, "--git-dir=")) { setenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1); if (envchanged) @@ -145,9 +144,8 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) (*argv)++; (*argc)--; - handled++; } - return handled; + return (*argv) - orig_argv; } static int handle_alias(int *argcp, const char ***argv) diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index 52c9ac9..de2a014 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -854,7 +854,7 @@ test_expect_success 'git -c "key=value" support' ' test_must_fail git -c core.name=value config name ' -test_expect_failure 'git -c works with aliases of builtins' ' +test_expect_success 'git -c works with aliases of builtins' ' git config alias.checkconfig "-c foo.check=bar config foo.check" && echo bar >expect && git checkconfig >actual && -- cgit v0.10.2-6-g49f6