@matheusbsbs / matheus-bagliano / commits / ce1a5a22a5

config: really pretend missing :(optional) value is not there

Earlier we added support for a value spelled as ":(optional)path" for configuration variables whose values are of type "path", with the documented semantics "if the path is missing, behave as if such a variable definition is not even there." This has worked OK for code paths that reads configuration files and stores the configured value as a string, where NULL in such a string is treated as if the setting is not there, left as the default. However, there are other code paths that do not _ignore_ such NULL values and misbehave. "git config get --path" is one of them. When git_config_pathname() helper function finds that the value of the variable is an optional path *and* the path is missing, it leaves the destination pointer intact (which usually is left to NULL) and returns 0 to signal a success. format_config() helper however assumed that the destination pointer always gets a string, which no longer is the case, and segfaulted. Make sure that git_config_pathname() clears the destination pointer in such a case, and teach format_config() to react to the condition by returning 1 (which is different from 0 that is a normal success and negative that is an error) to its callers. Adjust the callers to react to this new return value that tells them to pretend as if they did not even see this partcular <key, value> pair. Reported-by: Han Jiang <jhcarl0814@gmail.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Nov 20, 2025 at 11:34 UTC ce1a5a22a5beefac8a52da518855b5aecc562874

4 files changed +76 -9

builtin/config.c

+36 -9

@@ -261,6 +261,12 @@ struct strbuf_list {

261 int alloc;

262 };

263

264 +/*

265 + * Format the configuration key-value pair (`key_`, `value_`) and

266 + * append it into strbuf `buf`. Returns a negative value on failure,

267 + * 0 on success, 1 on a missing optional value (i.e., telling the

268 + * caller to pretend that <key_,value_> did not exist).

269 + */

270 static int format_config(const struct config_display_options *opts,

271 struct strbuf *buf, const char *key_,

272 const char *value_, const struct key_value_info *kvi)

@@ -299,7 +305,10 @@ static int format_config(const struct config_display_options *opts,

305 char *v;

306 if (git_config_pathname(&v, key_, value_) < 0)

307 return -1;

302 - strbuf_addstr(buf, v);

308 + if (v)

309 + strbuf_addstr(buf, v);

310 + else

311 + return 1; /* :(optional)no-such-file */

312 free((char *)v);

313 } else if (opts->type == TYPE_EXPIRY_DATE) {

314 timestamp_t t;

@@ -344,6 +353,7 @@ static int collect_config(const char *key_, const char *value_,

353 struct collect_config_data *data = cb;

354 struct strbuf_list *values = data->values;

355 const struct key_value_info *kvi = ctx->kvi;

356 + int status;

357

358 if (!(data->get_value_flags & GET_VALUE_KEY_REGEXP) &&

359 strcmp(key_, data->key))

@@ -361,8 +371,15 @@ static int collect_config(const char *key_, const char *value_,

371 ALLOC_GROW(values->items, values->nr + 1, values->alloc);

372 strbuf_init(&values->items[values->nr], 0);

373

364 - return format_config(data->display_opts, &values->items[values->nr++],

365 - key_, value_, kvi);

374 + status = format_config(data->display_opts, &values->items[values->nr++],

375 + key_, value_, kvi);

376 + if (status < 0)

377 + return status;

378 + if (status) {

379 + strbuf_release(&values->items[--values->nr]);

380 + status = 0;

381 + }

382 + return status;

383 }

384

385 static int get_value(const struct config_location_options *opts,

@@ -438,15 +455,23 @@ static int get_value(const struct config_location_options *opts,

455 if (!values.nr && display_opts->default_value) {

456 struct key_value_info kvi = KVI_INIT;

457 struct strbuf *item;

458 + int status;

459

460 kvi_from_param(&kvi);

461 ALLOC_GROW(values.items, values.nr + 1, values.alloc);

462 item = &values.items[values.nr++];

463 strbuf_init(item, 0);

446 - if (format_config(display_opts, item, key_,

447 - display_opts->default_value, &kvi) < 0)

464 +

465 + status = format_config(display_opts, item, key_,

466 + display_opts->default_value, &kvi);

467 + if (status < 0)

468 die(_("failed to format default config value: %s"),

469 display_opts->default_value);

470 + if (status) {

471 + /* default was a missing optional value */

472 + values.nr--;

473 + strbuf_release(item);

474 + }

475 }

476

477 ret = !values.nr;

@@ -706,11 +731,13 @@ static int get_urlmatch(const struct config_location_options *opts,

731 for_each_string_list_item(item, &values) {

732 struct urlmatch_current_candidate_value *matched = item->util;

733 struct strbuf buf = STRBUF_INIT;

734 + int status;

735

710 - format_config(&display_opts, &buf, item->string,

711 - matched->value_is_null ? NULL : matched->value.buf,

712 - &matched->kvi);

713 - fwrite(buf.buf, 1, buf.len, stdout);

736 + status = format_config(&display_opts, &buf, item->string,

737 + matched->value_is_null ? NULL : matched->value.buf,

738 + &matched->kvi);

739 + if (!status)

740 + fwrite(buf.buf, 1, buf.len, stdout);

741 strbuf_release(&buf);

742

743 strbuf_release(&matched->value);

config.c

@@ -1292,6 +1292,7 @@ int git_config_pathname(char **dest, const char *var, const char *value)

1292

1293 if (is_optional && is_missing_file(path)) {

1294 free(path);

1295 + *dest = NULL;

1296 return 0;

1297 }

1298

t/meson.build

@@ -182,6 +182,7 @@ integration_tests = [

182 't1308-config-set.sh',

183 't1309-early-config.sh',

184 't1310-config-default.sh',

185 + 't1311-config-optional.sh',

186 't1350-config-hooks-path.sh',

187 't1400-update-ref.sh',

188 't1401-symbolic-ref.sh',

t/t1311-config-optional.sh new

+38

@@ -0,0 +1,38 @@

1 +#!/bin/sh

2 +#

4 +#

5 +

6 +test_description=':(optional) paths'

7 +

8 +. ./test-lib.sh

9 +

10 +test_expect_success 'var=:(optional)path-exists' '

11 + test_config a.path ":(optional)path-exists" &&

12 + >path-exists &&

13 + echo path-exists >expect &&

14 +

15 + git config get --path a.path >actual &&

16 + test_cmp expect actual

17 +'

18 +

19 +test_expect_success 'missing optional value is ignored' '

20 + test_config a.path ":(optional)no-such-path" &&

21 + # Using --show-scope ensures we skip writing not only the value

22 + # but also any meta-information about the ignored key.

23 + test_must_fail git config get --show-scope --path a.path >actual &&

24 + test_line_count = 0 actual

25 +'

26 +

27 +test_expect_success 'missing optional value is ignored in multi-value config' '

28 + test_when_finished "git config unset --all a.path" &&

29 + git config set --append a.path ":(optional)path-exists" &&

30 + git config set --append a.path ":(optional)no-such-path" &&

31 + >path-exists &&

32 + echo path-exists >expect &&

33 +

34 + git config --get --path a.path >actual &&

35 + test_cmp expect actual

36 +'

37 +

38 +test_done