@matheusbsbs / matheus-bagliano / commits / a428619309

config API: have *_multi() return an "int" and take a "dest"

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 take advantage of 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. For now we don't make use of those new return values, but faithfully convert existing API users. Most of this is straightforward, commentary on cases that stand out: - To ensure that we'll properly use the return values of this function in the future we're using the "RESULT_MUST_BE_USED" macro introduced in [1]. As git_die_config() now has to handle this return value let's have it BUG() if it can't find the config entry. As tested for in a preceding commit we can rely on getting the config list in git_die_config(). - 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. - In "versioncmp.c" we now use the return value of the functions, instead of checking if the lists are still non-NULL. 1. 1e8697b5c4e (submodule--helper: check repo{_submodule,}_init() return values, 2022-09-01), Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Mar 28, 2023 at 16:04 UTC a428619309f42b76b5f750e66a5c1fd2225db3b3

9 files changed +64 -46

builtin/for-each-repo.c

+1 -4

@@ -45,14 +45,11 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)

45 if (!config_key)

46 die(_("missing --config=<config>"));

48 - values = repo_config_get_value_multi(the_repository,

49 - config_key);

50 -

48 /*

49 * Do nothing on an empty list, which is equivalent to the case

50 * where the config variable does not exist at all.

51 */

55 - if (!values)

52 + if (repo_config_get_value_multi(the_repository, config_key, &values))

53 return 0;

55 for (i = 0; !result && i < values->nr; i++)

builtin/gc.c

+4 -6

@@ -1510,8 +1510,7 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)

1510 if (git_config_get("maintenance.strategy"))

1511 git_config_set("maintenance.strategy", "incremental");

1512

1513 - list = git_config_get_value_multi(key);

1514 - if (list) {

1513 + if (!git_config_get_value_multi(key, &list)) {

1514 for_each_string_list_item(item, list) {

1515 if (!strcmp(maintpath, item->string)) {

1516 found = 1;

@@ -1577,11 +1576,10 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi

1576 if (config_file) {

1577 git_configset_init(&cs);

1578 git_configset_add_file(&cs, config_file);

1580 - list = git_configset_get_value_multi(&cs, key);

1581 - } else {

1582 - list = git_config_get_value_multi(key);

1579 }

1584 - if (list) {

1580 + if (!(config_file

1581 + ? git_configset_get_value_multi(&cs, key, &list)

1582 + : git_config_get_value_multi(key, &list))) {

1583 for_each_string_list_item(item, list) {

1584 if (!strcmp(maintpath, item->string)) {

1585 found = 1;

builtin/log.c

+3 -3

@@ -182,10 +182,10 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f

182 int i;

183 char *value = NULL;

184 struct string_list *include = decoration_filter->include_ref_pattern;

185 - const struct string_list *config_exclude =

186 - git_config_get_value_multi("log.excludeDecoration");

185 + const struct string_list *config_exclude;

186

188 - if (config_exclude) {

187 + if (!git_config_get_value_multi("log.excludeDecoration",

188 + &config_exclude)) {

189 struct string_list_item *item;

190 for_each_string_list_item(item, config_exclude)

191 string_list_append(decoration_filter->exclude_ref_config_pattern,

config.c

+20 -14

@@ -2404,29 +2404,34 @@ int git_configset_add_file(struct config_set *cs, const char *filename)

2404 int git_configset_get_value(struct config_set *cs, const char *key, const char **value)

2405 {

2406 const struct string_list *values = NULL;

2407 + int ret;

2408 +

2409 /*

2410 * Follows "last one wins" semantic, i.e., if there are multiple matches for the

2411 * queried key in the files of the configset, the value returned will be the last

2412 * value in the value list for that key.

2413 */

2412 - values = git_configset_get_value_multi(cs, key);

2414 + if ((ret = git_configset_get_value_multi(cs, key, &values)))

2415 + return ret;

2416

2414 - if (!values)

2415 - return 1;

2417 assert(values->nr > 0);

2418 *value = values->items[values->nr - 1].string;

2419 return 0;

2420 }

2421

2421 -const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key)

2422 +int git_configset_get_value_multi(struct config_set *cs, const char *key,

2423 + const struct string_list **dest)

2424 {

2425 struct config_set_element *e;

2426 + int ret;

2427

2425 - if (configset_find_element(cs, key, &e))

2426 - return NULL;

2428 + if ((ret = configset_find_element(cs, key, &e)))

2429 + return ret;

2430 else if (!e)

2428 - return NULL;

2429 - return &e->value_list;

2431 + return 1;

2432 + *dest = &e->value_list;

2433 +

2434 + return 0;

2435 }

2436

2437 int git_configset_get(struct config_set *cs, const char *key)

@@ -2590,11 +2595,11 @@ int repo_config_get_value(struct repository *repo,

2595 return git_configset_get_value(repo->config, key, value);

2596 }

2597

2593 -const struct string_list *repo_config_get_value_multi(struct repository *repo,

2594 - const char *key)

2598 +int repo_config_get_value_multi(struct repository *repo, const char *key,

2599 + const struct string_list **dest)

2600 {

2601 git_config_check_init(repo);

2597 - return git_configset_get_value_multi(repo->config, key);

2602 + return git_configset_get_value_multi(repo->config, key, dest);

2603 }

2604

2605 int repo_config_get_string(struct repository *repo,

@@ -2707,9 +2712,9 @@ int git_config_get_value(const char *key, const char **value)

2712 return repo_config_get_value(the_repository, key, value);

2713 }

2714

2710 -const struct string_list *git_config_get_value_multi(const char *key)

2715 +int git_config_get_value_multi(const char *key, const struct string_list **dest)

2716 {

2712 - return repo_config_get_value_multi(the_repository, key);

2717 + return repo_config_get_value_multi(the_repository, key, dest);

2718 }

2719

2720 int git_config_get_string(const char *key, char **dest)

@@ -2856,7 +2861,8 @@ void git_die_config(const char *key, const char *err, ...)

2861 error_fn(err, params);

2862 va_end(params);

2863 }

2859 - values = git_config_get_value_multi(key);

2864 + if (git_config_get_value_multi(key, &values))

2865 + BUG("for key '%s' we must have a value to report on", key);

2866 kv_info = values->items[values->nr - 1].util;

2867 git_die_config_linenr(key, kv_info->filename, kv_info->linenr);

2868 }

config.h

+21 -8

@@ -459,10 +459,18 @@ int git_configset_add_parameters(struct config_set *cs);

459 /**

460 * Finds and returns the value list, sorted in order of increasing priority

461 * for the configuration variable `key` and config set `cs`. When the

462 - * configuration variable `key` is not found, returns NULL. The caller

463 - * should not free or modify the returned pointer, as it is owned by the cache.

462 + * configuration variable `key` is not found, returns 1 without touching

463 + * `value`.

464 + *

465 + * The key will be parsed for validity with git_config_parse_key(), on

466 + * error a negative value will be returned.

467 + *

468 + * The caller should not free or modify the returned pointer, as it is

469 + * owned by the cache.

470 */

465 -const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key);

471 +RESULT_MUST_BE_USED

472 +int git_configset_get_value_multi(struct config_set *cs, const char *key,

473 + const struct string_list **dest);

474

475 /**

476 * Clears `config_set` structure, removes all saved variable-value pairs.

@@ -511,8 +519,9 @@ RESULT_MUST_BE_USED

519 int repo_config_get(struct repository *repo, const char *key);

520 int repo_config_get_value(struct repository *repo,

521 const char *key, const char **value);

514 -const struct string_list *repo_config_get_value_multi(struct repository *repo,

515 - const char *key);

522 +RESULT_MUST_BE_USED

523 +int repo_config_get_value_multi(struct repository *repo, const char *key,

524 + const struct string_list **dest);

525 int repo_config_get_string(struct repository *repo,

526 const char *key, char **dest);

527 int repo_config_get_string_tmp(struct repository *repo,

@@ -566,10 +575,14 @@ int git_config_get_value(const char *key, const char **value);

575 /**

576 * Finds and returns the value list, sorted in order of increasing priority

577 * for the configuration variable `key`. When the configuration variable

569 - * `key` is not found, returns NULL. The caller should not free or modify

570 - * the returned pointer, as it is owned by the cache.

578 + * `key` is not found, returns 1 without touching `value`.

579 + *

580 + * The caller should not free or modify the returned pointer, as it is

581 + * owned by the cache.

582 */

572 -const struct string_list *git_config_get_value_multi(const char *key);

583 +RESULT_MUST_BE_USED

584 +int git_config_get_value_multi(const char *key,

585 + const struct string_list **dest);

586

587 /**

588 * Resets and invalidates the config cache.

pack-bitmap.c

+5 -1

@@ -2301,7 +2301,11 @@ int bitmap_is_midx(struct bitmap_index *bitmap_git)

2301

2302 const struct string_list *bitmap_preferred_tips(struct repository *r)

2303 {

2304 - return repo_config_get_value_multi(r, "pack.preferbitmaptips");

2304 + const struct string_list *dest;

2305 +

2306 + if (!repo_config_get_value_multi(r, "pack.preferbitmaptips", &dest))

2307 + return dest;

2308 + return NULL;

2309 }

2310

2311 int bitmap_is_preferred_refname(struct repository *r, const char *refname)

submodule.c

+1 -2

@@ -274,8 +274,7 @@ int is_tree_submodule_active(struct repository *repo,

274 free(key);

275

276 /* submodule.active is set */

277 - sl = repo_config_get_value_multi(repo, "submodule.active");

278 - if (sl) {

277 + if (!repo_config_get_value_multi(repo, "submodule.active", &sl)) {

278 struct pathspec ps;

279 struct strvec args = STRVEC_INIT;

280 const struct string_list_item *item;

t/helper/test-config.c

+2 -4

@@ -97,8 +97,7 @@ int cmd__config(int argc, const char **argv)

97 goto exit1;

98 }

99 } else if (argc == 3 && !strcmp(argv[1], "get_value_multi")) {

100 - strptr = git_config_get_value_multi(argv[2]);

101 - if (strptr) {

100 + if (!git_config_get_value_multi(argv[2], &strptr)) {

101 for (i = 0; i < strptr->nr; i++) {

102 v = strptr->items[i].string;

103 if (!v)

@@ -181,8 +180,7 @@ int cmd__config(int argc, const char **argv)

180 goto exit2;

181 }

182 }

184 - strptr = git_configset_get_value_multi(&cs, argv[2]);

185 - if (strptr) {

183 + if (!git_configset_get_value_multi(&cs, argv[2], &strptr)) {

184 for (i = 0; i < strptr->nr; i++) {

185 v = strptr->items[i].string;

186 if (!v)

versioncmp.c

+7 -4

@@ -162,13 +162,16 @@ int versioncmp(const char *s1, const char *s2)

162 if (!initialized) {

163 const char *const newk = "versionsort.suffix";

164 const char *const oldk = "versionsort.prereleasesuffix";

165 + const struct string_list *newl;

166 const struct string_list *oldl;

167 + int new = git_config_get_value_multi(newk, &newl);

168 + int old = git_config_get_value_multi(oldk, &oldl);

169

167 - prereleases = git_config_get_value_multi(newk);

168 - oldl = git_config_get_value_multi(oldk);

169 - if (prereleases && oldl)

170 + if (!new && !old)

171 warning("ignoring %s because %s is set", oldk, newk);

171 - else if (!prereleases)

172 + if (!new)

173 + prereleases = newl;

174 + else if (!old)

175 prereleases = oldl;

176

177 initialized = 1;