config: report cached filenames in die_bad_number()
If, when parsing numbers from config, die_bad_number() is called, it reports the filename and config source type if we were parsing a config file, but not if we were iterating a config_set (it defaults to a less specific error message). Most call sites don't parse config files because config is typically read once and cached, so we only report filename and config source type in "git config --type" (since "git config" always parses config files). This could have been fixed when we taught the current_config_* functions to respect config_set values (0d44a2dacc (config: return configset value for current_config_ functions, 2016-05-26), but it was hard to spot then and we might have just missed it (I didn't find mention of die_bad_number() in the original ML discussion [1].) Fix this by refactoring the current_config_* functions into variants that don't BUG() when we aren't reading config, and using the resulting functions in die_bad_number(). "git config --get[-regexp] --type=int" cannot use the non-refactored version because it parses the int value _after_ parsing the config file, which would run into the BUG(). Since the refactored functions aren't public, they use "struct config_reader". 1. https://lore.kernel.org/git/20160518223712.GA18317@sigill.intra.peff.net/ Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Glen Choo committed
Mar 28, 2023 at 17:51 UTC
e2016508e7690cddc789fb28879703082363549d
4 files changed
+72
-20
config.c
+45
-20
@@ -1311,39 +1311,48 @@ int git_parse_ssize_t(const char *value, ssize_t *ret)
1311
return 1;
1312
}
1313
1314
+static int reader_config_name(struct config_reader *reader, const char **out);
1315
+static int reader_origin_type(struct config_reader *reader,
1316
+ enum config_origin_type *type);
1317
NORETURN
1315
-static void die_bad_number(struct config_source *cf, const char *name,
1318
+static void die_bad_number(struct config_reader *reader, const char *name,
1319
const char *value)
1320
{
1321
const char *error_type = (errno == ERANGE) ?
1322
N_("out of range") : N_("invalid unit");
1323
const char *bad_numeric = N_("bad numeric config value '%s' for '%s': %s");
1324
+ const char *config_name = NULL;
1325
+ enum config_origin_type config_origin = CONFIG_ORIGIN_UNKNOWN;
1326
1327
if (!value)
1328
value = "";
1329
1325
- if (!(cf && cf->name))
1330
+ /* Ignoring the return value is okay since we handle missing values. */
1331
+ reader_config_name(reader, &config_name);
1332
+ reader_origin_type(reader, &config_origin);
1333
+
1334
+ if (!config_name)
1335
die(_(bad_numeric), value, name, _(error_type));
1336
1328
- switch (cf->origin_type) {
1337
+ switch (config_origin) {
1338
case CONFIG_ORIGIN_BLOB:
1339
die(_("bad numeric config value '%s' for '%s' in blob %s: %s"),
1331
- value, name, cf->name, _(error_type));
1340
+ value, name, config_name, _(error_type));
1341
case CONFIG_ORIGIN_FILE:
1342
die(_("bad numeric config value '%s' for '%s' in file %s: %s"),
1334
- value, name, cf->name, _(error_type));
1343
+ value, name, config_name, _(error_type));
1344
case CONFIG_ORIGIN_STDIN:
1345
die(_("bad numeric config value '%s' for '%s' in standard input: %s"),
1346
value, name, _(error_type));
1347
case CONFIG_ORIGIN_SUBMODULE_BLOB:
1348
die(_("bad numeric config value '%s' for '%s' in submodule-blob %s: %s"),
1340
- value, name, cf->name, _(error_type));
1349
+ value, name, config_name, _(error_type));
1350
case CONFIG_ORIGIN_CMDLINE:
1351
die(_("bad numeric config value '%s' for '%s' in command line %s: %s"),
1343
- value, name, cf->name, _(error_type));
1352
+ value, name, config_name, _(error_type));
1353
default:
1354
die(_("bad numeric config value '%s' for '%s' in %s: %s"),
1346
- value, name, cf->name, _(error_type));
1355
+ value, name, config_name, _(error_type));
1356
}
1357
}
1358
@@ -1351,7 +1360,7 @@ int git_config_int(const char *name, const char *value)
1360
{
1361
int ret;
1362
if (!git_parse_int(value, &ret))
1354
- die_bad_number(the_reader.source, name, value);
1363
+ die_bad_number(&the_reader, name, value);
1364
return ret;
1365
}
1366
@@ -1359,7 +1368,7 @@ int64_t git_config_int64(const char *name, const char *value)
1368
{
1369
int64_t ret;
1370
if (!git_parse_int64(value, &ret))
1362
- die_bad_number(the_reader.source, name, value);
1371
+ die_bad_number(&the_reader, name, value);
1372
return ret;
1373
}
1374
@@ -1367,7 +1376,7 @@ unsigned long git_config_ulong(const char *name, const char *value)
1376
{
1377
unsigned long ret;
1378
if (!git_parse_ulong(value, &ret))
1370
- die_bad_number(the_reader.source, name, value);
1379
+ die_bad_number(&the_reader, name, value);
1380
return ret;
1381
}
1382
@@ -1375,7 +1384,7 @@ ssize_t git_config_ssize_t(const char *name, const char *value)
1384
{
1385
ssize_t ret;
1386
if (!git_parse_ssize_t(value, &ret))
1378
- die_bad_number(the_reader.source, name, value);
1387
+ die_bad_number(&the_reader, name, value);
1388
return ret;
1389
}
1390
@@ -3839,14 +3848,23 @@ int parse_config_key(const char *var,
3848
return 0;
3849
}
3850
3842
-const char *current_config_origin_type(void)
3851
+static int reader_origin_type(struct config_reader *reader,
3852
+ enum config_origin_type *type)
3853
{
3844
- int type;
3854
if (the_reader.config_kvi)
3846
- type = the_reader.config_kvi->origin_type;
3855
+ *type = reader->config_kvi->origin_type;
3856
else if(the_reader.source)
3848
- type = the_reader.source->origin_type;
3857
+ *type = reader->source->origin_type;
3858
else
3859
+ return 1;
3860
+ return 0;
3861
+}
3862
+
3863
+const char *current_config_origin_type(void)
3864
+{
3865
+ enum config_origin_type type = CONFIG_ORIGIN_UNKNOWN;
3866
+
3867
+ if (reader_origin_type(&the_reader, &type))
3868
BUG("current_config_origin_type called outside config callback");
3869
3870
switch (type) {
@@ -3885,14 +3903,21 @@ const char *config_scope_name(enum config_scope scope)
3903
}
3904
}
3905
3888
-const char *current_config_name(void)
3906
+static int reader_config_name(struct config_reader *reader, const char **out)
3907
{
3890
- const char *name;
3908
if (the_reader.config_kvi)
3892
- name = the_reader.config_kvi->filename;
3909
+ *out = reader->config_kvi->filename;
3910
else if (the_reader.source)
3894
- name = the_reader.source->name;
3911
+ *out = reader->source->name;
3912
else
3913
+ return 1;
3914
+ return 0;
3915
+}
3916
+
3917
+const char *current_config_name(void)
3918
+{
3919
+ const char *name;
3920
+ if (reader_config_name(&the_reader, &name))
3921
BUG("current_config_name called outside config callback");
3922
return name ? name : "";
3923
}
config.h
+1
@@ -56,6 +56,7 @@ struct git_config_source {
56
};
57
58
enum config_origin_type {
59
+ CONFIG_ORIGIN_UNKNOWN = 0,
60
CONFIG_ORIGIN_BLOB,
61
CONFIG_ORIGIN_FILE,
62
CONFIG_ORIGIN_STDIN,
t/helper/test-config.c
+17
@@ -30,6 +30,9 @@
30
* iterate -> iterate over all values using git_config(), and print some
31
* data for each
32
*
33
+ * git_config_int -> iterate over all values using git_config() and print the
34
+ * integer value for the entered key or die
35
+ *
36
* Examples:
37
*
38
* To print the value with highest priority for key "foo.bAr Baz.rock":
@@ -54,6 +57,17 @@ static int iterate_cb(const char *var, const char *value, void *data UNUSED)
57
return 0;
58
}
59
60
+static int parse_int_cb(const char *var, const char *value, void *data)
61
+{
62
+ const char *key_to_match = data;
63
+
64
+ if (!strcmp(key_to_match, var)) {
65
+ int parsed = git_config_int(value, value);
66
+ printf("%d\n", parsed);
67
+ }
68
+ return 0;
69
+}
70
+
71
static int early_config_cb(const char *var, const char *value, void *vdata)
72
{
73
const char *key = vdata;
@@ -176,6 +190,9 @@ int cmd__config(int argc, const char **argv)
190
} else if (!strcmp(argv[1], "iterate")) {
191
git_config(iterate_cb, NULL);
192
goto exit0;
193
+ } else if (argc == 3 && !strcmp(argv[1], "git_config_int")) {
194
+ git_config(parse_int_cb, (void *) argv[2]);
195
+ goto exit0;
196
}
197
198
die("%s: Please check the syntax and the function name", argv[0]);
t/t1308-config-set.sh
+9
@@ -120,6 +120,10 @@ test_expect_success 'find integer value for a key' '
120
check_config get_int lamb.chop 65
121
'
122
123
+test_expect_success 'parse integer value during iteration' '
124
+ check_config git_config_int lamb.chop 65
125
+'
126
+
127
test_expect_success 'find string value for a key' '
128
check_config get_string case.baz hask &&
129
check_config expect_code 1 get_string case.ba "Value not found for \"case.ba\""
@@ -134,6 +138,11 @@ test_expect_success 'find integer if value is non parse-able' '
138
check_config expect_code 128 get_int lamb.head
139
'
140
141
+test_expect_success 'non parse-able integer value during iteration' '
142
+ check_config expect_code 128 git_config_int lamb.head 2>result &&
143
+ grep "fatal: bad numeric config value .* in file \.git/config" result
144
+'
145
+
146
test_expect_success 'find bool value for the entered key' '
147
check_config get_bool goat.head 1 &&
148
check_config get_bool goat.skin 0 &&