From 47db054df053fb09c8c92edaa0238af2a2605e65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 13 Nov 2012 13:41:01 -0800 Subject: [PATCH 1/2] config: distinguish between a lone variable name and one without rhs '[section] variable' and '[section] variable =' behave differently when parsed as booleans, so we need to store that distinction internally. --- src/config_file.c | 4 +++- src/remote.c | 2 +- src/util.c | 11 ++++------- tests-clar/config/read.c | 6 ++++++ tests-clar/resources/config/config4 | 2 ++ 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/config_file.c b/src/config_file.c index 4ca842b89..4d9f99986 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -1432,8 +1432,10 @@ static int parse_variable(diskfile_backend *cfg, char **var_name, char **var_val else if (value_start[0] != '\0') { *var_value = fixup_line(value_start, 0); GITERR_CHECK_ALLOC(*var_value); + } else { /* equals sign but missing rhs */ + *var_value = git__strdup(""); + GITERR_CHECK_ALLOC(*var_value); } - } git__free(line); diff --git a/src/remote.c b/src/remote.c index 8c46ca6a1..70a615246 100644 --- a/src/remote.c +++ b/src/remote.c @@ -136,7 +136,7 @@ int git_remote_load(git_remote **out, git_repository *repo, const char *name) if ((error = git_config_get_string(&val, config, git_buf_cstr(&buf))) < 0) goto cleanup; - if (!val) { + if (!val || strlen(val) == 0) { giterr_set(GITERR_INVALID, "Malformed remote '%s' - missing URL", name); error = -1; goto cleanup; diff --git a/src/util.c b/src/util.c index 0a82ccea6..7f5043817 100644 --- a/src/util.c +++ b/src/util.c @@ -432,12 +432,8 @@ int git__strcmp_cb(const void *a, const void *b) int git__parse_bool(int *out, const char *value) { /* A missing value means true */ - if (value == NULL) { - *out = 1; - return 0; - } - - if (!strcasecmp(value, "true") || + if (value == NULL || + !strcasecmp(value, "true") || !strcasecmp(value, "yes") || !strcasecmp(value, "on")) { *out = 1; @@ -445,7 +441,8 @@ int git__parse_bool(int *out, const char *value) } if (!strcasecmp(value, "false") || !strcasecmp(value, "no") || - !strcasecmp(value, "off")) { + !strcasecmp(value, "off") || + value[0] == '\0') { *out = 0; return 0; } diff --git a/tests-clar/config/read.c b/tests-clar/config/read.c index 10ae0a4fb..a468a4d92 100644 --- a/tests-clar/config/read.c +++ b/tests-clar/config/read.c @@ -93,6 +93,12 @@ void test_config_read__lone_variable(void) cl_git_pass(git_config_get_bool(&i, cfg, "some.section.variable")); cl_assert(i == 1); + cl_git_pass(git_config_get_string(&str, cfg, "some.section.variableeq")); + cl_assert_equal_s(str, ""); + + cl_git_pass(git_config_get_bool(&i, cfg, "some.section.variableeq")); + cl_assert(i == 0); + git_config_free(cfg); } diff --git a/tests-clar/resources/config/config4 b/tests-clar/resources/config/config4 index 741fa0ffd..9dd40419e 100644 --- a/tests-clar/resources/config/config4 +++ b/tests-clar/resources/config/config4 @@ -1,3 +1,5 @@ # A variable name on its own is valid [some.section] variable +# A variable and '=' is accepted, but it's not considered true + variableeq = From 0da81d2b39290fe4d444953acb6d68795ed1ef42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 13 Nov 2012 14:43:23 -0800 Subject: [PATCH 2/2] config: return an emtpy string when there is no value Returning NULL for the string when we haven't signaled an error condition is counter-intuitive and causes unnecessary edge cases. Return an empty string when asking for a string value for a configuration variable such as '[section] var' to avoid these edge cases. If the distinction between no value and an empty value is needed, this can be retrieved from the entry directly. As a side-effect, this change stops the int parsing functions from segfaulting on such a variable. --- src/config.c | 38 ++++++++++++++++++++++++-------------- src/remote.c | 6 ++++-- tests-clar/config/read.c | 4 +++- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/config.c b/src/config.c index ed9901bd2..4fb161169 100644 --- a/src/config.c +++ b/src/config.c @@ -393,24 +393,11 @@ int git_config_get_int32(int32_t *out, git_config *cfg, const char *name) return git_config_parse_int32(out, value); } -int git_config_get_bool(int *out, git_config *cfg, const char *name) -{ - const char *value; - int ret; - - if ((ret = git_config_get_string(&value, cfg, name)) < 0) - return ret; - - return git_config_parse_bool(out, value); -} - static int get_string_at_file(const char **out, git_config_file *file, const char *name) { const git_config_entry *entry; int res; - *out = NULL; - res = file->get(file, name, &entry); if (!res) *out = entry->value; @@ -418,7 +405,7 @@ static int get_string_at_file(const char **out, git_config_file *file, const cha return res; } -int git_config_get_string(const char **out, git_config *cfg, const char *name) +static int get_string(const char **out, git_config *cfg, const char *name) { file_internal *internal; unsigned int i; @@ -435,6 +422,29 @@ int git_config_get_string(const char **out, git_config *cfg, const char *name) return GIT_ENOTFOUND; } +int git_config_get_bool(int *out, git_config *cfg, const char *name) +{ + const char *value; + int ret; + + if ((ret = get_string(&value, cfg, name)) < 0) + return ret; + + return git_config_parse_bool(out, value); +} + +int git_config_get_string(const char **out, git_config *cfg, const char *name) +{ + int ret; + const char *str; + + if ((ret = get_string(&str, cfg, name)) < 0) + return ret; + + *out = str == NULL ? "" : str; + return 0; +} + int git_config_get_entry(const git_config_entry **out, git_config *cfg, const char *name) { file_internal *internal; diff --git a/src/remote.c b/src/remote.c index 70a615246..4a4d160eb 100644 --- a/src/remote.c +++ b/src/remote.c @@ -136,7 +136,7 @@ int git_remote_load(git_remote **out, git_repository *repo, const char *name) if ((error = git_config_get_string(&val, config, git_buf_cstr(&buf))) < 0) goto cleanup; - if (!val || strlen(val) == 0) { + if (strlen(val) == 0) { giterr_set(GITERR_INVALID, "Malformed remote '%s' - missing URL", name); error = -1; goto cleanup; @@ -153,8 +153,10 @@ int git_remote_load(git_remote **out, git_repository *repo, const char *name) } error = git_config_get_string(&val, config, git_buf_cstr(&buf)); - if (error == GIT_ENOTFOUND) + if (error == GIT_ENOTFOUND) { + val = NULL; error = 0; + } if (error < 0) { error = -1; diff --git a/tests-clar/config/read.c b/tests-clar/config/read.c index a468a4d92..7b30b6e12 100644 --- a/tests-clar/config/read.c +++ b/tests-clar/config/read.c @@ -87,8 +87,10 @@ void test_config_read__lone_variable(void) cl_git_pass(git_config_open_ondisk(&cfg, cl_fixture("config/config4"))); + cl_git_fail(git_config_get_int32(&i, cfg, "some.section.variable")); + cl_git_pass(git_config_get_string(&str, cfg, "some.section.variable")); - cl_assert(str == NULL); + cl_assert_equal_s(str, ""); cl_git_pass(git_config_get_bool(&i, cfg, "some.section.variable")); cl_assert(i == 1);