@@ -32,6 +32,7 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
static const char *config_key = NULL;
int i, result = 0;
const struct string_list *values;
+ int err;
const struct option options[] = {
OPT_STRING(0, "config", &config_key, N_("config"),
@@ -45,14 +46,10 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
if (!config_key)
die(_("missing --config=<config>"));
- values = repo_config_get_value_multi(the_repository,
- config_key);
-
- /*
- * Do nothing on an empty list, which is equivalent to the case
- * where the config variable does not exist at all.
- */
- if (!values)
+ err = repo_config_get_value_multi(the_repository, config_key, &values);
+ if (err < 0)
+ return 0;
+ else if (err)
return 0;
for (i = 0; !result && i < values->nr; i++)
@@ -1513,8 +1513,7 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)
else
git_config_set("maintenance.strategy", "incremental");
- list = git_config_get_value_multi(key);
- if (list) {
+ if (!git_config_get_value_multi(key, &list)) {
for_each_string_list_item(item, list) {
if (!strcmp(maintpath, item->string)) {
found = 1;
@@ -1580,11 +1579,10 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
if (config_file) {
git_configset_init(&cs);
git_configset_add_file(&cs, config_file);
- list = git_configset_get_value_multi(&cs, key);
- } else {
- list = git_config_get_value_multi(key);
}
- if (list) {
+ if (!(config_file
+ ? git_configset_get_value_multi(&cs, key, &list)
+ : git_config_get_value_multi(key, &list))) {
for_each_string_list_item(item, list) {
if (!strcmp(maintpath, item->string)) {
found = 1;
@@ -182,10 +182,10 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f
int i;
char *value = NULL;
struct string_list *include = decoration_filter->include_ref_pattern;
- const struct string_list *config_exclude =
- git_config_get_value_multi("log.excludeDecoration");
+ const struct string_list *config_exclude;
- if (config_exclude) {
+ if (!git_config_get_value_multi("log.excludeDecoration",
+ &config_exclude)) {
struct string_list_item *item;
for_each_string_list_item(item, config_exclude)
string_list_append(decoration_filter->exclude_ref_config_pattern,
@@ -541,6 +541,7 @@ static int module_init(int argc, const char **argv, const char *prefix)
NULL
};
int ret = 1;
+ const struct string_list *values;
argc = parse_options(argc, argv, prefix, module_init_options,
git_submodule_helper_usage, 0);
@@ -552,7 +553,7 @@ static int module_init(int argc, const char **argv, const char *prefix)
* If there are no path args and submodule.active is set then,
* by default, only initialize 'active' modules.
*/
- if (!argc && git_config_get_value_multi("submodule.active"))
+ if (!argc && !git_config_get_value_multi("submodule.active", &values))
module_list_active(&list);
info.prefix = prefix;
@@ -2714,6 +2715,7 @@ static int module_update(int argc, const char **argv, const char *prefix)
if (opt.init) {
struct module_list list = MODULE_LIST_INIT;
struct init_cb info = INIT_CB_INIT;
+ const struct string_list *values;
if (module_list_compute(argv, opt.prefix,
&pathspec2, &list) < 0) {
@@ -2726,7 +2728,8 @@ static int module_update(int argc, const char **argv, const char *prefix)
* If there are no path args and submodule.active is set then,
* by default, only initialize 'active' modules.
*/
- if (!argc && git_config_get_value_multi("submodule.active"))
+ if (!argc && !git_config_get_value_multi("submodule.active",
+ &values))
module_list_active(&list);
info.prefix = opt.prefix;
@@ -2275,23 +2275,28 @@ void read_very_early_config(config_fn_t cb, void *data)
config_with_options(cb, data, NULL, &opts);
}
-static struct config_set_element *configset_find_element(struct config_set *cs, const char *key)
+static int configset_find_element(struct config_set *cs, const char *key,
+ struct config_set_element **dest)
{
struct config_set_element k;
struct config_set_element *found_entry;
char *normalized_key;
+ int ret;
+
/*
* `key` may come from the user, so normalize it before using it
* for querying entries from the hashmap.
*/
- if (git_config_parse_key(key, &normalized_key, NULL))
- return NULL;
+ ret = git_config_parse_key(key, &normalized_key, NULL);
+ if (ret < 0)
+ return ret;
hashmap_entry_init(&k.ent, strhash(normalized_key));
k.key = normalized_key;
found_entry = hashmap_get_entry(&cs->config_hash, &k, ent, NULL);
free(normalized_key);
- return found_entry;
+ *dest = found_entry;
+ return 0;
}
static int configset_add_value(struct config_set *cs, const char *key, const char *value)
@@ -2300,8 +2305,11 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha
struct string_list_item *si;
struct configset_list_item *l_item;
struct key_value_info *kv_info = xmalloc(sizeof(*kv_info));
+ int ret;
- e = configset_find_element(cs, key);
+ ret = configset_find_element(cs, key, &e);
+ if (ret < 0)
+ return ret;
/*
* Since the keys are being fed by git_config*() callback mechanism, they
* are already normalized. So simply add them without any further munging.
@@ -2395,24 +2403,38 @@ int git_configset_add_file(struct config_set *cs, const char *filename)
int git_configset_get_value(struct config_set *cs, const char *key, const char **value)
{
const struct string_list *values = NULL;
+ int ret;
+
/*
* Follows "last one wins" semantic, i.e., if there are multiple matches for the
* queried key in the files of the configset, the value returned will be the last
* value in the value list for that key.
*/
- values = git_configset_get_value_multi(cs, key);
+ ret = git_configset_get_value_multi(cs, key, &values);
- if (!values)
+ if (ret < 0)
+ return ret;
+ else if (!values)
return 1;
assert(values->nr > 0);
*value = values->items[values->nr - 1].string;
return 0;
}
-const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key)
+int git_configset_get_value_multi(struct config_set *cs, const char *key,
+ const struct string_list **dest)
{
- struct config_set_element *e = configset_find_element(cs, key);
- return e ? &e->value_list : NULL;
+ struct config_set_element *e;
+ int ret;
+
+ ret = configset_find_element(cs, key, &e);
+ if (ret < 0)
+ return ret;
+ else if (!e)
+ return 1;
+ *dest = &e->value_list;
+
+ return 0;
}
int git_configset_get_string(struct config_set *cs, const char *key, char **dest)
@@ -2558,11 +2580,11 @@ int repo_config_get_value(struct repository *repo,
return git_configset_get_value(repo->config, key, value);
}
-const struct string_list *repo_config_get_value_multi(struct repository *repo,
- const char *key)
+int repo_config_get_value_multi(struct repository *repo, const char *key,
+ const struct string_list **dest)
{
git_config_check_init(repo);
- return git_configset_get_value_multi(repo->config, key);
+ return git_configset_get_value_multi(repo->config, key, dest);
}
int repo_config_get_string(struct repository *repo,
@@ -2670,9 +2692,9 @@ int git_config_get_value(const char *key, const char **value)
return repo_config_get_value(the_repository, key, value);
}
-const struct string_list *git_config_get_value_multi(const char *key)
+int git_config_get_value_multi(const char *key, const struct string_list **dest)
{
- return repo_config_get_value_multi(the_repository, key);
+ return repo_config_get_value_multi(the_repository, key, dest);
}
int git_config_get_string(const char *key, char **dest)
@@ -2819,7 +2841,8 @@ void git_die_config(const char *key, const char *err, ...)
error_fn(err, params);
va_end(params);
}
- values = git_config_get_value_multi(key);
+ if (git_config_get_value_multi(key, &values))
+ BUG("for key '%s' we must have a value to report on", key);
kv_info = values->items[values->nr - 1].util;
git_die_config_linenr(key, kv_info->filename, kv_info->linenr);
}
@@ -459,10 +459,18 @@ int git_configset_add_parameters(struct config_set *cs);
/**
* Finds and returns the value list, sorted in order of increasing priority
* for the configuration variable `key` and config set `cs`. When the
- * configuration variable `key` is not found, returns NULL. The caller
- * should not free or modify the returned pointer, as it is owned by the cache.
+ * configuration variable `key` is not found, returns 1 without touching
+ * `value`.
+ *
+ * The key will be parsed for validity with git_config_parse_key(), on
+ * error a negative value will be returned.
+ *
+ * The caller should not free or modify the returned pointer, as it is
+ * owned by the cache.
*/
-const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key);
+RESULT_MUST_BE_USED
+int git_configset_get_value_multi(struct config_set *cs, const char *key,
+ const struct string_list **dest);
/**
* Clears `config_set` structure, removes all saved variable-value pairs.
@@ -496,8 +504,9 @@ struct repository;
void repo_config(struct repository *repo, config_fn_t fn, void *data);
int repo_config_get_value(struct repository *repo,
const char *key, const char **value);
-const struct string_list *repo_config_get_value_multi(struct repository *repo,
- const char *key);
+RESULT_MUST_BE_USED
+int repo_config_get_value_multi(struct repository *repo, const char *key,
+ const struct string_list **dest);
int repo_config_get_string(struct repository *repo,
const char *key, char **dest);
int repo_config_get_string_tmp(struct repository *repo,
@@ -544,10 +553,14 @@ int git_config_get_value(const char *key, const char **value);
/**
* Finds and returns the value list, sorted in order of increasing priority
* for the configuration variable `key`. When the configuration variable
- * `key` is not found, returns NULL. The caller should not free or modify
- * the returned pointer, as it is owned by the cache.
+ * `key` is not found, returns 1 without touching `value`.
+ *
+ * The caller should not free or modify the returned pointer, as it is
+ * owned by the cache.
*/
-const struct string_list *git_config_get_value_multi(const char *key);
+RESULT_MUST_BE_USED
+int git_config_get_value_multi(const char *key,
+ const struct string_list **dest);
/**
* Resets and invalidates the config cache.
@@ -2301,7 +2301,11 @@ int bitmap_is_midx(struct bitmap_index *bitmap_git)
const struct string_list *bitmap_preferred_tips(struct repository *r)
{
- return repo_config_get_value_multi(r, "pack.preferbitmaptips");
+ const struct string_list *dest;
+
+ if (!repo_config_get_value_multi(r, "pack.preferbitmaptips", &dest))
+ return dest;
+ return NULL;
}
int bitmap_is_preferred_refname(struct repository *r, const char *refname)
@@ -274,8 +274,7 @@ int is_tree_submodule_active(struct repository *repo,
free(key);
/* submodule.active is set */
- sl = repo_config_get_value_multi(repo, "submodule.active");
- if (sl) {
+ if (!repo_config_get_value_multi(repo, "submodule.active", &sl)) {
struct pathspec ps;
struct strvec args = STRVEC_INIT;
const struct string_list_item *item;
@@ -95,8 +95,7 @@ int cmd__config(int argc, const char **argv)
goto exit1;
}
} else if (argc == 3 && !strcmp(argv[1], "get_value_multi")) {
- strptr = git_config_get_value_multi(argv[2]);
- if (strptr) {
+ if (!git_config_get_value_multi(argv[2], &strptr)) {
for (i = 0; i < strptr->nr; i++) {
v = strptr->items[i].string;
if (!v)
@@ -159,8 +158,7 @@ int cmd__config(int argc, const char **argv)
goto exit2;
}
}
- strptr = git_configset_get_value_multi(&cs, argv[2]);
- if (strptr) {
+ if (!git_configset_get_value_multi(&cs, argv[2], &strptr)) {
for (i = 0; i < strptr->nr; i++) {
v = strptr->items[i].string;
if (!v)
@@ -162,13 +162,16 @@ int versioncmp(const char *s1, const char *s2)
if (!initialized) {
const char *const newk = "versionsort.suffix";
const char *const oldk = "versionsort.prereleasesuffix";
+ const struct string_list *newl;
const struct string_list *oldl;
+ int new = git_config_get_value_multi(newk, &newl);
+ int old = git_config_get_value_multi(oldk, &oldl);
- prereleases = git_config_get_value_multi(newk);
- oldl = git_config_get_value_multi(oldk);
- if (prereleases && oldl)
+ if (!new && !old)
warning("ignoring %s because %s is set", oldk, newk);
- else if (!prereleases)
+ if (!new)
+ prereleases = newl;
+ else if (!old)
prereleases = oldl;
initialized = 1;
Have the "git_configset_get_value_multi()" function and its siblings return an "int" and populate a "**dest" parameter like every other git_configset_get_*()" in the API. As we'll see in in subsequent commits this fixes a blind spot in the API where it wasn't possible to tell whether a list was empty from whether a config key existed. We'll take advantage of that in subsequent commits, but for now we're faithfully converting existing API callers. See [1] for the initial addition of "git_configset_get_value_multi()" 1. 3c8687a73ee (add `config_set` API for caching config-like files, 2014-07-28). A logical follow-up to this would be to change the various "*_get_*()" functions to ferry the git_configset_get_value() return value to their own callers, e.g. git_configset_get_int() returns "1" rather than ferrying up the "-1" that "git_configset_get_value()" might return, but that's not being done in this series Most of this is straightforward, commentary on cases that stand out: - As we've tested for in a preceding commit we can rely on getting the config list in git_die_config(), and as we need to handle the new return value let's BUG() out if we can't acquire it. - In "builtin/for-each-ref.c" we could preserve the comment added in 6c62f015520, but now that we're directly using the documented repo_config_get_value_multi() value it's just narrating something that should be obvious from the API use, so let's drop it. - The loops after getting the "list" value in "builtin/gc.c" could also make use of "unsorted_string_list_has_string()" instead of using that loop, but let's leave that for now. - We have code e.g. in "builtin/submodule--helper.c" that only wants to check if a config key exists, and would be better served with another API, but let's keep using "git_configset_get_value_multi()" for now. - In "versioncmp.c" we now use the return value of the functions, instead of checking if the lists are still non-NULL. This is strictly speaking unnecessary, but makes the API use consistent with the rest, but more importantly... - ...because we always check our return values we can assert that with the RESULT_MUST_BE_USED macro added in 1e8697b5c4e (submodule--helper: check repo{_submodule,}_init() return values, 2022-09-01) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- builtin/for-each-repo.c | 13 ++++----- builtin/gc.c | 10 +++---- builtin/log.c | 6 ++-- builtin/submodule--helper.c | 7 +++-- config.c | 55 ++++++++++++++++++++++++++----------- config.h | 29 +++++++++++++------ pack-bitmap.c | 6 +++- submodule.c | 3 +- t/helper/test-config.c | 6 ++-- versioncmp.c | 11 +++++--- 10 files changed, 92 insertions(+), 54 deletions(-)