Re: [PATCH v2 5/6] config: add `git_die_config()` to the config-set API
Tanay Abhra writes: > On 7/25/2014 7:33 PM, Matthieu Moy wrote: >> Tanay Abhra writes: >> >>> --- a/config.c >>> +++ b/config.c >>> @@ -1403,11 +1403,12 @@ const struct string_list >>> *git_configset_get_value_multi(struct config_set *cs, c >>> >>> int git_configset_get_string_const(struct config_set *cs, const char *key, >>> const char **dest) >>> { >>> - const char *value; >>> - if (!git_configset_get_value(cs, key, &value)) >>> - return git_config_string(dest, key, value); >>> - else >>> - return 1; >>> + int ret; >>> + char *value; >>> + ret = git_configset_get_string(cs, key, &value); >>> + if (ret <= 0) >>> + *dest = (const char*)value; >>> + return ret; >>> } >> >> Isn't this a fixup meant for another series? >> > > Though v12 is in pu, Junio commented that git_configset_get_string_const() & > git_configset_get_string() can be done more concisely, I was trying to do > that but I failed. My comment on that version was not about conciseness. You had one that called git_config_string() to let the callee do the nonbool error handling and xstrdup() of the non-error return value, and the other one that did exactly what a call to git_config_string() would have done. That is being redundant, not just failing to be concise. I was actually hoping that we would see just int git_configset_get_string(struct config_set *cs, const char *key, char **dest) { return git_configset_get_string_const(cs, key, (const char **)dest); } with the implementation of _const() variant be the one from v12. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/6] config: add `git_die_config()` to the config-set API
On 7/25/2014 7:33 PM, Matthieu Moy wrote: > Tanay Abhra writes: > >> --- a/config.c >> +++ b/config.c >> @@ -1403,11 +1403,12 @@ const struct string_list >> *git_configset_get_value_multi(struct config_set *cs, c >> >> int git_configset_get_string_const(struct config_set *cs, const char *key, >> const char **dest) >> { >> -const char *value; >> -if (!git_configset_get_value(cs, key, &value)) >> -return git_config_string(dest, key, value); >> -else >> -return 1; >> +int ret; >> +char *value; >> +ret = git_configset_get_string(cs, key, &value); >> +if (ret <= 0) >> +*dest = (const char*)value; >> +return ret; >> } > > Isn't this a fixup meant for another series? > Though v12 is in pu, Junio commented that git_configset_get_string_const() & git_configset_get_string() can be done more concisely, I was trying to do that but I failed. >> int git_configset_get_string(struct config_set *cs, const char *key, char >> **dest) >> @@ -1418,8 +1419,7 @@ int git_configset_get_string(struct config_set *cs, >> const char *key, char **dest >> return config_error_nonbool(key); >> *dest = xstrdup(value); >> return 0; >> -} >> -else >> +} else >> return 1; > > Useless churn. > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/6] config: add `git_die_config()` to the config-set API
Tanay Abhra writes: > --- a/config.c > +++ b/config.c > @@ -1403,11 +1403,12 @@ const struct string_list > *git_configset_get_value_multi(struct config_set *cs, c > > int git_configset_get_string_const(struct config_set *cs, const char *key, > const char **dest) > { > - const char *value; > - if (!git_configset_get_value(cs, key, &value)) > - return git_config_string(dest, key, value); > - else > - return 1; > + int ret; > + char *value; > + ret = git_configset_get_string(cs, key, &value); > + if (ret <= 0) > + *dest = (const char*)value; > + return ret; > } Isn't this a fixup meant for another series? > int git_configset_get_string(struct config_set *cs, const char *key, char > **dest) > @@ -1418,8 +1419,7 @@ int git_configset_get_string(struct config_set *cs, > const char *key, char **dest > return config_error_nonbool(key); > *dest = xstrdup(value); > return 0; > - } > - else > + } else > return 1; Useless churn. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/6] config: add `git_die_config()` to the config-set API
Add `git_die_config` that dies printing the line number and the file name of the highest priority value for the configuration variable `key`. It has usage in non-callback based config value retrieval where we can raise an error and die if there is a semantic error. For example, if (!git_config_get_value(key, &value)) { /* NULL values not allowed */ if (!value) git_config_die(key); else /* do work */ } Signed-off-by: Tanay Abhra --- Documentation/technical/api-config.txt | 5 cache.h| 1 + config.c | 44 ++ 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 815c1ee..e7ec7cc 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -155,6 +155,11 @@ as well as retrieval for the queried variable, including: Similar to `git_config_get_string`, but expands `~` or `~user` into the user's home directory when found at the beginning of the path. +`void git_die_config(const char *key)`:: + + Dies printing the line number and the file name of the highest + priority value for the configuration variable `key`. + See test-config.c for usage examples. Value Parsing Helpers diff --git a/cache.h b/cache.h index 11ded5a..5c3dd88 100644 --- a/cache.h +++ b/cache.h @@ -1407,6 +1407,7 @@ extern int git_config_get_bool(const char *key, int *dest); extern int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest); extern int git_config_get_maybe_bool(const char *key, int *dest); extern int git_config_get_pathname(const char *key, const char **dest); +extern void git_die_config(const char *key); extern int committer_ident_sufficiently_given(void); extern int author_ident_sufficiently_given(void); diff --git a/config.c b/config.c index aa5c0ad..1e49ae7 100644 --- a/config.c +++ b/config.c @@ -1403,11 +1403,12 @@ const struct string_list *git_configset_get_value_multi(struct config_set *cs, c int git_configset_get_string_const(struct config_set *cs, const char *key, const char **dest) { - const char *value; - if (!git_configset_get_value(cs, key, &value)) - return git_config_string(dest, key, value); - else - return 1; + int ret; + char *value; + ret = git_configset_get_string(cs, key, &value); + if (ret <= 0) + *dest = (const char*)value; + return ret; } int git_configset_get_string(struct config_set *cs, const char *key, char **dest) @@ -1418,8 +1419,7 @@ int git_configset_get_string(struct config_set *cs, const char *key, char **dest return config_error_nonbool(key); *dest = xstrdup(value); return 0; - } - else + } else return 1; } @@ -1514,14 +1514,22 @@ const struct string_list *git_config_get_value_multi(const char *key) int git_config_get_string_const(const char *key, const char **dest) { + int ret; git_config_check_init(); - return git_configset_get_string_const(&the_config_set, key, dest); + ret = git_configset_get_string_const(&the_config_set, key, dest); + if (ret < 0) + git_die_config(key); + return ret; } int git_config_get_string(const char *key, char **dest) { + int ret; git_config_check_init(); - return git_configset_get_string(&the_config_set, key, dest); + ret = git_configset_get_string(&the_config_set, key, dest); + if (ret < 0) + git_die_config(key); + return ret; } int git_config_get_int(const char *key, int *dest) @@ -1556,8 +1564,24 @@ int git_config_get_maybe_bool(const char *key, int *dest) int git_config_get_pathname(const char *key, const char **dest) { + int ret; git_config_check_init(); - return git_configset_get_pathname(&the_config_set, key, dest); + ret = git_configset_get_pathname(&the_config_set, key, dest); + if (ret < 0) + git_die_config(key); + return ret; +} + +void git_die_config(const char *key) +{ + const struct string_list *values; + struct key_value_info *kv_info; + values = git_config_get_value_multi(key); + kv_info = values->items[values->nr - 1].util; + if (!kv_info->linenr) + die("unable to parse command-line config"); + else + die("bad config file line %d in %s",kv_info->linenr, kv_info->filename); } /* -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html