From 8c3ca351cb125c94870d7b8278a7efa1f1474d22 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 9 Apr 2016 13:42:31 -0400 Subject: [PATCH 1/3] config: lower-case first word of error strings This follows our usual style (both throughout git, and throughout the rest of this file). This covers the whole file, but note that I left the capitalization in the multi-sentence: error: malformed value... error: Must be one of ... because it helps make it clear that we are starting a new sentence in the second one. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- config.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/config.c b/config.c index 9ba40bc1b039b9..92b0cf447fa161 100644 --- a/config.c +++ b/config.c @@ -108,7 +108,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc expanded = expand_user_path(path); if (!expanded) - return error("Could not expand include path '%s'", path); + return error("could not expand include path '%s'", path); path = expanded; /* @@ -950,7 +950,7 @@ static int git_default_branch_config(const char *var, const char *value) else if (!strcmp(value, "always")) autorebase = AUTOREBASE_ALWAYS; else - return error("Malformed value for %s", var); + return error("malformed value for %s", var); return 0; } @@ -976,7 +976,7 @@ static int git_default_push_config(const char *var, const char *value) else if (!strcmp(value, "current")) push_default = PUSH_DEFAULT_CURRENT; else { - error("Malformed value for %s: %s", var, value); + error("malformed value for %s: %s", var, value); return error("Must be one of nothing, matching, simple, " "upstream or current."); } @@ -2223,7 +2223,7 @@ void git_config_set_multivar_in_file(const char *config_filename, { if (git_config_set_multivar_in_file_gently(config_filename, key, value, value_regex, multi_replace) < 0) - die(_("Could not set '%s' to '%s'"), key, value); + die(_("could not set '%s' to '%s'"), key, value); } int git_config_set_multivar_gently(const char *key, const char *value, @@ -2404,7 +2404,7 @@ int git_config_rename_section(const char *old_name, const char *new_name) #undef config_error_nonbool int config_error_nonbool(const char *var) { - return error("Missing value for '%s'", var); + return error("missing value for '%s'", var); } int parse_config_key(const char *var, From 9c14bb08a434570adc9b2f0f37eac66b92d4c87e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 9 Apr 2016 13:42:54 -0400 Subject: [PATCH 2/3] git_config_set_multivar_in_file: all non-zero returns are errors This function is just a thin wrapper for the "_gently" form of the function. But the gently form is designed to feed builtin/config.c, which passes our return code directly to its exit status, and thus uses positive error values for some cases. We check only negative values, meaning we would fail to die in some cases (e.g., a malformed key). This may or may not be triggerable in practice; we tend to use this non-gentle form only when setting internal variables, which would not have malformed keys. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.c b/config.c index 92b0cf447fa161..d349d52f26365b 100644 --- a/config.c +++ b/config.c @@ -2222,7 +2222,7 @@ void git_config_set_multivar_in_file(const char *config_filename, const char *value_regex, int multi_replace) { if (git_config_set_multivar_in_file_gently(config_filename, key, value, - value_regex, multi_replace) < 0) + value_regex, multi_replace)) die(_("could not set '%s' to '%s'"), key, value); } From 1cae428e2902b3f19a56625411f09cc239855fe7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 9 Apr 2016 13:43:54 -0400 Subject: [PATCH 3/3] git_config_set_multivar_in_file: handle "unset" errors We pass off to the "_gently" form to do the real work, and just die() if it returned an error. However, our die message de-references "value", which may be NULL if the request was to unset a variable. Nobody using glibc noticed, because it simply prints "(null)", which is good enough for the test suite (and presumably very few people run across this in practice). But other libc implementations (like Solaris) may segfault. Let's not only fix that, but let's make the message more clear about what is going on in the "unset" case. Reported-by: "Tom G. Christensen" Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- config.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/config.c b/config.c index d349d52f26365b..68a2f939efc8f3 100644 --- a/config.c +++ b/config.c @@ -2221,9 +2221,13 @@ void git_config_set_multivar_in_file(const char *config_filename, const char *key, const char *value, const char *value_regex, int multi_replace) { - if (git_config_set_multivar_in_file_gently(config_filename, key, value, - value_regex, multi_replace)) + if (!git_config_set_multivar_in_file_gently(config_filename, key, value, + value_regex, multi_replace)) + return; + if (value) die(_("could not set '%s' to '%s'"), key, value); + else + die(_("could not unset '%s'"), key); } int git_config_set_multivar_gently(const char *key, const char *value,