@@ -362,8 +362,7 @@ static int get_value(const char *key_, const char *regex_, unsigned flags)
goto free_strings;
}
} else {
- if (git_config_parse_key(key_, &key, NULL)) {
- ret = CONFIG_INVALID_KEY;
+ if ((ret = git_config_parse_key(key_, &key, NULL))) {
goto free_strings;
}
}
@@ -534,8 +534,9 @@ static inline int iskeychar(int c)
* Auxiliary function to sanity-check and split the key into the section
* identifier and variable name.
*
- * Returns 0 on success, -1 when there is an invalid character in the key and
- * -2 if there is no section name in the key.
+ * Returns 0 on success, CONFIG_INVALID_KEY when there is an invalid character
+ * in the key and CONFIG_NO_SECTION_OR_NAME if there is no section name in the
+ * key.
*
* store_key - pointer to char* which will hold a copy of the key with
* lowercase section and variable name
@@ -555,12 +556,12 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
if (last_dot == NULL || last_dot == key) {
error(_("key does not contain a section: %s"), key);
- return -CONFIG_NO_SECTION_OR_NAME;
+ return CONFIG_NO_SECTION_OR_NAME;
}
if (!last_dot[1]) {
error(_("key does not contain variable name: %s"), key);
- return -CONFIG_NO_SECTION_OR_NAME;
+ return CONFIG_NO_SECTION_OR_NAME;
}
baselen = last_dot - key;
@@ -596,7 +597,7 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
out_free_ret_1:
FREE_AND_NULL(*store_key);
- return -CONFIG_INVALID_KEY;
+ return CONFIG_INVALID_KEY;
}
static int config_parse_pair(const char *key, const char *value,
@@ -2346,7 +2347,7 @@ static int configset_find_element(struct config_set *set, const char *key,
* `key` may come from the user, so normalize it before using it
* for querying entries from the hashmap.
*/
- ret = git_config_parse_key(key, &normalized_key, NULL);
+ ret = -git_config_parse_key(key, &normalized_key, NULL);
if (ret)
return ret;
@@ -3334,8 +3335,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
size_t contents_sz;
struct config_store_data store = CONFIG_STORE_INIT;
- /* parse-key returns negative; flip the sign to feed exit(3) */
- ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
+ ret = git_config_parse_key(key, &store.key, &store.baselen);
if (ret)
goto out_free;
@@ -23,7 +23,7 @@
struct object_id;
-/* git_config_parse_key() returns these negated: */
+/* git_config_parse_key() returns these: */
#define CONFIG_INVALID_KEY 1
#define CONFIG_NO_SECTION_OR_NAME 2
/* git_config_set_gently(), git_config_set_multivar_gently() return the above or these: */
@@ -824,8 +824,8 @@ int print_config_from_gitmodules(struct repository *repo, const char *key)
char *store_key;
ret = git_config_parse_key(key, &store_key, NULL);
- if (ret < 0)
- return CONFIG_INVALID_KEY;
+ if (ret)
+ return ret;
config_from_gitmodules(config_print_callback, repo, store_key);
@@ -2590,4 +2590,20 @@ test_expect_success 'includeIf.hasconfig:remote.*.url forbids remote url in such
grep "fatal: remote URLs cannot be configured in file directly or indirectly included by includeIf.hasconfig:remote.*.url" err
'
+# Exit codes
+test_expect_success '--get with bad key' '
+ # Also exits with 1 if the value is not found
+ test_expect_code 1 git config --get "bad.name\n" 2>err &&
+ grep "error: invalid key" err &&
+ test_expect_code 2 git config --get "bad." 2>err &&
+ grep "error: key does not contain variable name" err
+'
+
+test_expect_success 'set with bad key' '
+ test_expect_code 1 git config "bad.name\n" var 2>err &&
+ grep "error: invalid key" err &&
+ test_expect_code 2 git config "bad." var 2>err &&
+ grep "error: key does not contain variable name" err
+'
+
test_done
git_config_parse_key() returns #define-d error codes, but negated. This negation is merely a convenience to other parts of config.c that don't bother inspecting the return value before passing it along. But: a) There's no good reason why those callers couldn't negate the value themselves. b) In other callers, this value eventually gets fed to exit(3), and those callers need to sanitize the negative value (and they sometimes do so lossily, by overriding the return value with CONFIG_INVALID_KEY). c) We want to move that into a separate library, and returning only negative values no longer makes as much sense. Change git_config_parse_key() to return positive values instead, and adjust callers accordingly. Callers that sanitize the negative sign for exit(3) now pass the return value opaquely, fixing a bug where "git config <key with no section or name>" results in a different exit code depending on whether we are setting or getting config. Callers that wanted to pass along a negative value now negate the return value themselves. Signed-off-by: Glen Choo <chooglen@google.com> --- builtin/config.c | 3 +-- config.c | 16 ++++++++-------- config.h | 2 +- submodule-config.c | 4 ++-- t/t1300-config.sh | 16 ++++++++++++++++ 5 files changed, 28 insertions(+), 13 deletions(-)