On Tue, May 17, 2011 at 12:38 PM, Michael J Gruber
<git@xxxxxxxxxxxxxxxxxxxx> wrote:
> The return codes of git_config_set() and friends are magic numbers right
> in the source. #define them in cache.h where the functions are declared,
Why defining them on cache.h? Just because the functions are declared
there? Is this a good reason? I think this pollute even more the
cache.h and these constants are not used outside of config.c. So I'd
move them back onto config.c. Maybe it's fine as is though.
> and use the constants in the source.
>
> Also, mention the resulting exit codes of "git config" in its man page
> (and complete the list).
>
> Signed-off-by: Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx>
> ---
> ÂDocumentation/git-config.txt | Â 18 ++++++++++--------
> Âcache.h           Â|  10 ++++++++++
> Âconfig.c           |  20 ++++++++++----------
> Â3 files changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> index 8804de3..e7ecf5d 100644
> --- a/Documentation/git-config.txt
> +++ b/Documentation/git-config.txt
> @@ -50,16 +50,18 @@ The default is to assume the config file of the current repository,
> Â.git/config unless defined otherwise with GIT_DIR and GIT_CONFIG
> Â(see <<FILES>>).
>
> -This command will fail if:
> -
> -. The config file is invalid,
> -. Can not write to the config file,
> -. no section was provided,
> -. the section or key is invalid,
> -. you try to unset an option which does not exist,
> -. you try to unset/set an option for which multiple lines match, or
> -. you use '--global' option without $HOME being properly set.
> -
> +This command will fail (with exit code ret) if:
> +
> +. The config file is invalid (ret=3),
> +. can not write to the config file (ret=4),
> +. no section or name was provided (ret=2),
> +. the section or key is invalid (ret=1),
> +. you try to unset an option which does not exist (ret=5),
> +. you try to unset/set an option for which multiple lines match (ret=5),
> +. you try to use an invalid regexp (ret=6), or
> +. you use '--global' option without $HOME being properly set (ret=128).
> +
> +On success, the command returns the exit code 0.
>
> ÂOPTIONS
> Â-------
> diff --git a/cache.h b/cache.h
> index 2225c3f..57a98d2 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1018,6 +1018,16 @@ extern const char *packed_object_info_detail(struct packed_git *, off_t, unsigne
> Â/* Dumb servers support */
> Âextern int update_server_info(int);
>
> +/* git_config_parse_key() returns these negated: */
> +#define CONFIG_INVALID_KEY 1
> +#define CONFIG_NO_SECTION_OR_NAME 2
> +/* git_config_set(), git_config_set_multivar() return the above or these: */
> +#define CONFIG_NO_LOCK -1
> +#define CONFIG_INVALID_FILE 3
> +#define CONFIG_NO_WRITE 4
> +#define CONFIG_NOTHING_SET 5
> +#define CONFIG_INVALID_PATTERN 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 *);
> diff --git a/config.c b/config.c
> index 671c8df..9d36848 100644
> --- a/config.c
> +++ b/config.c
> @@ -1123,12 +1123,12 @@ int git_config_parse_key(const char *key, char **store_key, int *baselen_)
>
> Â Â Â Âif (last_dot == NULL || last_dot == key) {
> Â Â Â Â Â Â Â Âerror("key does not contain a section: %s", key);
> - Â Â Â Â Â Â Â return -2;
> + Â Â Â Â Â Â Â return -CONFIG_NO_SECTION_OR_NAME;
> Â Â Â Â}
>
> Â Â Â Âif (!last_dot[1]) {
> Â Â Â Â Â Â Â Âerror("key does not contain variable name: %s", key);
> - Â Â Â Â Â Â Â return -2;
> + Â Â Â Â Â Â Â return -CONFIG_NO_SECTION_OR_NAME;
> Â Â Â Â}
>
> Â Â Â Âbaselen = last_dot - key;
> @@ -1165,7 +1165,7 @@ int git_config_parse_key(const char *key, char **store_key, int *baselen_)
>
> Âout_free_ret_1:
> Â Â Â Âfree(*store_key);
> - Â Â Â return -1;
> + Â Â Â return -CONFIG_INVALID_KEY;
> Â}
>
> Â/*
> @@ -1221,7 +1221,7 @@ int git_config_set_multivar(const char *key, const char *value,
> Â Â Â Âif (fd < 0) {
> Â Â Â Â Â Â Â Âerror("could not lock config file %s: %s", config_filename, strerror(errno));
> Â Â Â Â Â Â Â Âfree(store.key);
> - Â Â Â Â Â Â Â ret = -1;
> + Â Â Â Â Â Â Â ret = CONFIG_NO_LOCK;
> Â Â Â Â Â Â Â Âgoto out_free;
> Â Â Â Â}
>
> @@ -1235,12 +1235,12 @@ int git_config_set_multivar(const char *key, const char *value,
> Â Â Â Â Â Â Â Âif ( ENOENT != errno ) {
> Â Â Â Â Â Â Â Â Â Â Â Âerror("opening %s: %s", config_filename,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âstrerror(errno));
> - Â Â Â Â Â Â Â Â Â Â Â ret = 3; /* same as "invalid config file" */
> + Â Â Â Â Â Â Â Â Â Â Â ret = CONFIG_INVALID_FILE; /* same as "invalid config file" */
> Â Â Â Â Â Â Â Â Â Â Â Âgoto out_free;
> Â Â Â Â Â Â Â Â}
> Â Â Â Â Â Â Â Â/* if nothing to unset, error out */
> Â Â Â Â Â Â Â Âif (value == NULL) {
> - Â Â Â Â Â Â Â Â Â Â Â ret = 5;
> + Â Â Â Â Â Â Â Â Â Â Â ret = CONFIG_NOTHING_SET;
> Â Â Â Â Â Â Â Â Â Â Â Âgoto out_free;
> Â Â Â Â Â Â Â Â}
>
> @@ -1268,7 +1268,7 @@ int git_config_set_multivar(const char *key, const char *value,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ÂREG_EXTENDED)) {
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âerror("invalid pattern: %s", value_regex);
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âfree(store.value_regex);
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ret = 6;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ret = CONFIG_INVALID_PATTERN;
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âgoto out_free;
> Â Â Â Â Â Â Â Â Â Â Â Â}
> Â Â Â Â Â Â Â Â}
> @@ -1290,7 +1290,7 @@ int git_config_set_multivar(const char *key, const char *value,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âregfree(store.value_regex);
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âfree(store.value_regex);
> Â Â Â Â Â Â Â Â Â Â Â Â}
> - Â Â Â Â Â Â Â Â Â Â Â ret = 3;
> + Â Â Â Â Â Â Â Â Â Â Â ret = CONFIG_INVALID_FILE;
> Â Â Â Â Â Â Â Â Â Â Â Âgoto out_free;
> Â Â Â Â Â Â Â Â}
>
> @@ -1303,7 +1303,7 @@ int git_config_set_multivar(const char *key, const char *value,
> Â Â Â Â Â Â Â Â/* if nothing to unset, or too many matches, error out */
> Â Â Â Â Â Â Â Âif ((store.seen == 0 && value == NULL) ||
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â(store.seen > 1 && multi_replace == 0)) {
> - Â Â Â Â Â Â Â Â Â Â Â ret = 5;
> + Â Â Â Â Â Â Â Â Â Â Â ret = CONFIG_NOTHING_SET;
> Â Â Â Â Â Â Â Â Â Â Â Âgoto out_free;
> Â Â Â Â Â Â Â Â}
>
> @@ -1364,7 +1364,7 @@ int git_config_set_multivar(const char *key, const char *value,
>
> Â Â Â Âif (commit_lock_file(lock) < 0) {
> Â Â Â Â Â Â Â Âerror("could not commit config file %s", config_filename);
> - Â Â Â Â Â Â Â ret = 4;
> + Â Â Â Â Â Â Â ret = CONFIG_NO_WRITE;
> Â Â Â Â Â Â Â Âgoto out_free;
> Â Â Â Â}
>
> --
> 1.7.5.1.514.gd181fb
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html