Skip to content

Commit 87daf40

Browse files
gitstergitster
committed
Merge branch 'ab/config-multi-and-nonbool'
Assorted config API updates. * ab/config-multi-and-nonbool: for-each-repo: with bad config, don't conflate <path> and <cmd> config API: add "string" version of *_value_multi(), fix segfaults config API users: test for *_get_value_multi() segfaults for-each-repo: error on bad --config config API: have *_multi() return an "int" and take a "dest" versioncmp.c: refactor config reading next commit config API: add and use a "git_config_get()" family of functions config tests: add "NULL" tests for *_get_value_multi() config tests: cover blind spots in git_die_config() tests
2 parents e9dffbc + 3611f74 commit 87daf40

21 files changed

+481
-72
lines changed

‎builtin/for-each-repo.c

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
3232
static const char *config_key = NULL;
3333
int i, result = 0;
3434
const struct string_list *values;
35+
int err;
3536

3637
const struct option options[] = {
3738
OPT_STRING(0, "config", &config_key, N_("config"),
@@ -45,14 +46,11 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
4546
if (!config_key)
4647
die(_("missing --config=<config>"));
4748

48-
values = repo_config_get_value_multi(the_repository,
49-
config_key);
50-
51-
/*
52-
* Do nothing on an empty list, which is equivalent to the case
53-
* where the config variable does not exist at all.
54-
*/
55-
if (!values)
49+
err = repo_config_get_string_multi(the_repository, config_key, &values);
50+
if (err < 0)
51+
usage_msg_optf(_("got bad config --config=%s"),
52+
for_each_repo_usage, options, config_key);
53+
else if (err)
5654
return 0;
5755

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

‎builtin/gc.c

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1494,7 +1494,6 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)
14941494
};
14951495
int found = 0;
14961496
const char *key = "maintenance.repo";
1497-
char *config_value;
14981497
char *maintpath = get_maintpath();
14991498
struct string_list_item *item;
15001499
const struct string_list *list;
@@ -1509,13 +1508,10 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)
15091508
git_config_set("maintenance.auto", "false");
15101509

15111510
/* Set maintenance strategy, if unset */
1512-
if (!git_config_get_string("maintenance.strategy", &config_value))
1513-
free(config_value);
1514-
else
1511+
if (git_config_get("maintenance.strategy"))
15151512
git_config_set("maintenance.strategy", "incremental");
15161513

1517-
list = git_config_get_value_multi(key);
1518-
if (list) {
1514+
if (!git_config_get_string_multi(key, &list)) {
15191515
for_each_string_list_item(item, list) {
15201516
if (!strcmp(maintpath, item->string)) {
15211517
found = 1;
@@ -1581,11 +1577,10 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
15811577
if (config_file) {
15821578
git_configset_init(&cs);
15831579
git_configset_add_file(&cs, config_file);
1584-
list = git_configset_get_value_multi(&cs, key);
1585-
} else {
1586-
list = git_config_get_value_multi(key);
15871580
}
1588-
if (list) {
1581+
if (!(config_file
1582+
? git_configset_get_string_multi(&cs, key, &list)
1583+
: git_config_get_string_multi(key, &list))) {
15891584
for_each_string_list_item(item, list) {
15901585
if (!strcmp(maintpath, item->string)) {
15911586
found = 1;

‎builtin/log.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,10 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f
185185
int i;
186186
char *value = NULL;
187187
struct string_list *include = decoration_filter->include_ref_pattern;
188-
const struct string_list *config_exclude =
189-
git_config_get_value_multi("log.excludeDecoration");
188+
const struct string_list *config_exclude;
190189

191-
if (config_exclude) {
190+
if (!git_config_get_string_multi("log.excludeDecoration",
191+
&config_exclude)) {
192192
struct string_list_item *item;
193193
for_each_string_list_item(item, config_exclude)
194194
string_list_append(decoration_filter->exclude_ref_config_pattern,

‎builtin/submodule--helper.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,7 @@ static int module_init(int argc, const char **argv, const char *prefix)
559559
* If there are no path args and submodule.active is set then,
560560
* by default, only initialize 'active' modules.
561561
*/
562-
if (!argc && git_config_get_value_multi("submodule.active"))
562+
if (!argc && !git_config_get("submodule.active"))
563563
module_list_active(&list);
564564

565565
info.prefix = prefix;
@@ -2745,7 +2745,7 @@ static int module_update(int argc, const char **argv, const char *prefix)
27452745
* If there are no path args and submodule.active is set then,
27462746
* by default, only initialize 'active' modules.
27472747
*/
2748-
if (!argc && git_config_get_value_multi("submodule.active"))
2748+
if (!argc && !git_config_get("submodule.active"))
27492749
module_list_active(&list);
27502750

27512751
info.prefix = opt.prefix;
@@ -3142,7 +3142,6 @@ static int config_submodule_in_gitmodules(const char *name, const char *var, con
31423142
static void configure_added_submodule(struct add_data *add_data)
31433143
{
31443144
char *key;
3145-
const char *val;
31463145
struct child_process add_submod = CHILD_PROCESS_INIT;
31473146
struct child_process add_gitmodules = CHILD_PROCESS_INIT;
31483147

@@ -3187,7 +3186,7 @@ static void configure_added_submodule(struct add_data *add_data)
31873186
* is_submodule_active(), since that function needs to find
31883187
* out the value of "submodule.active" again anyway.
31893188
*/
3190-
if (!git_config_get_string_tmp("submodule.active", &val)) {
3189+
if (!git_config_get("submodule.active")) {
31913190
/*
31923191
* If the submodule being added isn't already covered by the
31933192
* current configured pathspec, set the submodule's active flag

‎builtin/worktree.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,6 @@ static void copy_filtered_worktree_config(const char *worktree_git_dir)
320320

321321
if (file_exists(from_file)) {
322322
struct config_set cs = { { 0 } };
323-
const char *core_worktree;
324323
int bare;
325324

326325
if (safe_create_leading_directories(to_file) ||
@@ -339,7 +338,7 @@ static void copy_filtered_worktree_config(const char *worktree_git_dir)
339338
to_file, "core.bare", NULL, "true", 0))
340339
error(_("failed to unset '%s' in '%s'"),
341340
"core.bare", to_file);
342-
if (!git_configset_get_value(&cs, "core.worktree", &core_worktree) &&
341+
if (!git_configset_get(&cs, "core.worktree") &&
343342
git_config_set_in_file_gently(to_file,
344343
"core.worktree", NULL))
345344
error(_("failed to unset '%s' in '%s'"),

‎config.c

Lines changed: 92 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2292,23 +2292,29 @@ void read_very_early_config(config_fn_t cb, void *data)
22922292
config_with_options(cb, data, NULL, &opts);
22932293
}
22942294

2295-
static struct config_set_element *configset_find_element(struct config_set *cs, const char *key)
2295+
RESULT_MUST_BE_USED
2296+
static int configset_find_element(struct config_set *cs, const char *key,
2297+
struct config_set_element **dest)
22962298
{
22972299
struct config_set_element k;
22982300
struct config_set_element *found_entry;
22992301
char *normalized_key;
2302+
int ret;
2303+
23002304
/*
23012305
* `key` may come from the user, so normalize it before using it
23022306
* for querying entries from the hashmap.
23032307
*/
2304-
if (git_config_parse_key(key, &normalized_key, NULL))
2305-
return NULL;
2308+
ret = git_config_parse_key(key, &normalized_key, NULL);
2309+
if (ret)
2310+
return ret;
23062311

23072312
hashmap_entry_init(&k.ent, strhash(normalized_key));
23082313
k.key = normalized_key;
23092314
found_entry = hashmap_get_entry(&cs->config_hash, &k, ent, NULL);
23102315
free(normalized_key);
2311-
return found_entry;
2316+
*dest = found_entry;
2317+
return 0;
23122318
}
23132319

23142320
static int configset_add_value(struct config_set *cs, const char *key, const char *value)
@@ -2317,8 +2323,11 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha
23172323
struct string_list_item *si;
23182324
struct configset_list_item *l_item;
23192325
struct key_value_info *kv_info = xmalloc(sizeof(*kv_info));
2326+
int ret;
23202327

2321-
e = configset_find_element(cs, key);
2328+
ret = configset_find_element(cs, key, &e);
2329+
if (ret)
2330+
return ret;
23222331
/*
23232332
* Since the keys are being fed by git_config*() callback mechanism, they
23242333
* are already normalized. So simply add them without any further munging.
@@ -2412,24 +2421,65 @@ int git_configset_add_file(struct config_set *cs, const char *filename)
24122421
int git_configset_get_value(struct config_set *cs, const char *key, const char **value)
24132422
{
24142423
const struct string_list *values = NULL;
2424+
int ret;
2425+
24152426
/*
24162427
* Follows "last one wins" semantic, i.e., if there are multiple matches for the
24172428
* queried key in the files of the configset, the value returned will be the last
24182429
* value in the value list for that key.
24192430
*/
2420-
values = git_configset_get_value_multi(cs, key);
2431+
if ((ret = git_configset_get_value_multi(cs, key, &values)))
2432+
return ret;
24212433

2422-
if (!values)
2423-
return 1;
24242434
assert(values->nr > 0);
24252435
*value = values->items[values->nr - 1].string;
24262436
return 0;
24272437
}
24282438

2429-
const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key)
2439+
int git_configset_get_value_multi(struct config_set *cs, const char *key,
2440+
const struct string_list **dest)
2441+
{
2442+
struct config_set_element *e;
2443+
int ret;
2444+
2445+
if ((ret = configset_find_element(cs, key, &e)))
2446+
return ret;
2447+
else if (!e)
2448+
return 1;
2449+
*dest = &e->value_list;
2450+
2451+
return 0;
2452+
}
2453+
2454+
static int check_multi_string(struct string_list_item *item, void *util)
2455+
{
2456+
return item->string ? 0 : config_error_nonbool(util);
2457+
}
2458+
2459+
int git_configset_get_string_multi(struct config_set *cs, const char *key,
2460+
const struct string_list **dest)
2461+
{
2462+
int ret;
2463+
2464+
if ((ret = git_configset_get_value_multi(cs, key, dest)))
2465+
return ret;
2466+
if ((ret = for_each_string_list((struct string_list *)*dest,
2467+
check_multi_string, (void *)key)))
2468+
return ret;
2469+
2470+
return 0;
2471+
}
2472+
2473+
int git_configset_get(struct config_set *cs, const char *key)
24302474
{
2431-
struct config_set_element *e = configset_find_element(cs, key);
2432-
return e ? &e->value_list : NULL;
2475+
struct config_set_element *e;
2476+
int ret;
2477+
2478+
if ((ret = configset_find_element(cs, key, &e)))
2479+
return ret;
2480+
else if (!e)
2481+
return 1;
2482+
return 0;
24332483
}
24342484

24352485
int git_configset_get_string(struct config_set *cs, const char *key, char **dest)
@@ -2568,18 +2618,31 @@ void repo_config(struct repository *repo, config_fn_t fn, void *data)
25682618
configset_iter(repo->config, fn, data);
25692619
}
25702620

2621+
int repo_config_get(struct repository *repo, const char *key)
2622+
{
2623+
git_config_check_init(repo);
2624+
return git_configset_get(repo->config, key);
2625+
}
2626+
25712627
int repo_config_get_value(struct repository *repo,
25722628
const char *key, const char **value)
25732629
{
25742630
git_config_check_init(repo);
25752631
return git_configset_get_value(repo->config, key, value);
25762632
}
25772633

2578-
const struct string_list *repo_config_get_value_multi(struct repository *repo,
2579-
const char *key)
2634+
int repo_config_get_value_multi(struct repository *repo, const char *key,
2635+
const struct string_list **dest)
2636+
{
2637+
git_config_check_init(repo);
2638+
return git_configset_get_value_multi(repo->config, key, dest);
2639+
}
2640+
2641+
int repo_config_get_string_multi(struct repository *repo, const char *key,
2642+
const struct string_list **dest)
25802643
{
25812644
git_config_check_init(repo);
2582-
return git_configset_get_value_multi(repo->config, key);
2645+
return git_configset_get_string_multi(repo->config, key, dest);
25832646
}
25842647

25852648
int repo_config_get_string(struct repository *repo,
@@ -2682,14 +2745,25 @@ void git_config_clear(void)
26822745
repo_config_clear(the_repository);
26832746
}
26842747

2748+
int git_config_get(const char *key)
2749+
{
2750+
return repo_config_get(the_repository, key);
2751+
}
2752+
26852753
int git_config_get_value(const char *key, const char **value)
26862754
{
26872755
return repo_config_get_value(the_repository, key, value);
26882756
}
26892757

2690-
const struct string_list *git_config_get_value_multi(const char *key)
2758+
int git_config_get_value_multi(const char *key, const struct string_list **dest)
2759+
{
2760+
return repo_config_get_value_multi(the_repository, key, dest);
2761+
}
2762+
2763+
int git_config_get_string_multi(const char *key,
2764+
const struct string_list **dest)
26912765
{
2692-
return repo_config_get_value_multi(the_repository, key);
2766+
return repo_config_get_string_multi(the_repository, key, dest);
26932767
}
26942768

26952769
int git_config_get_string(const char *key, char **dest)
@@ -2836,7 +2910,8 @@ void git_die_config(const char *key, const char *err, ...)
28362910
error_fn(err, params);
28372911
va_end(params);
28382912
}
2839-
values = git_config_get_value_multi(key);
2913+
if (git_config_get_value_multi(key, &values))
2914+
BUG("for key '%s' we must have a value to report on", key);
28402915
kv_info = values->items[values->nr - 1].util;
28412916
git_die_config_linenr(key, kv_info->filename, kv_info->linenr);
28422917
}

0 commit comments

Comments
 (0)