Skip to content

Commit fee8572

Browse files
dschogitster
dscho
authored and
gitster
committed
config: avoid using the global variable store
It is much easier to reason about, when the config code to set/unset variables or to remove/rename sections does not rely on a global (or file-local) variable. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 8032cc4 commit fee8572

File tree

1 file changed

+66
-53
lines changed

1 file changed

+66
-53
lines changed

‎config.c‎

Lines changed: 66 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2288,7 +2288,7 @@ void git_die_config(const char *key, const char *err, ...)
22882288
* Find all the stuff for git_config_set() below.
22892289
*/
22902290

2291-
static struct {
2291+
struct config_store_data {
22922292
int baselen;
22932293
char *key;
22942294
int do_not_match;
@@ -2298,83 +2298,86 @@ static struct {
22982298
unsigned int offset_alloc;
22992299
enum { START, SECTION_SEEN, SECTION_END_SEEN, KEY_SEEN } state;
23002300
unsigned int seen;
2301-
} store;
2301+
};
23022302

2303-
static int matches(const char *key, const char *value)
2303+
static int matches(const char *key, const char *value,
2304+
const struct config_store_data *store)
23042305
{
2305-
if (strcmp(key, store.key))
2306+
if (strcmp(key, store->key))
23062307
return 0; /* not ours */
2307-
if (!store.value_regex)
2308+
if (!store->value_regex)
23082309
return 1; /* always matches */
2309-
if (store.value_regex == CONFIG_REGEX_NONE)
2310+
if (store->value_regex == CONFIG_REGEX_NONE)
23102311
return 0; /* never matches */
23112312

2312-
return store.do_not_match ^
2313-
(value && !regexec(store.value_regex, value, 0, NULL, 0));
2313+
return store->do_not_match ^
2314+
(value && !regexec(store->value_regex, value, 0, NULL, 0));
23142315
}
23152316

23162317
static int store_aux(const char *key, const char *value, void *cb)
23172318
{
23182319
const char *ep;
23192320
size_t section_len;
2321+
struct config_store_data *store = cb;
23202322

2321-
switch (store.state) {
2323+
switch (store->state) {
23222324
case KEY_SEEN:
2323-
if (matches(key, value)) {
2324-
if (store.seen == 1 && store.multi_replace == 0) {
2325+
if (matches(key, value, store)) {
2326+
if (store->seen == 1 && store->multi_replace == 0) {
23252327
warning(_("%s has multiple values"), key);
23262328
}
23272329

2328-
ALLOC_GROW(store.offset, store.seen + 1,
2329-
store.offset_alloc);
2330+
ALLOC_GROW(store->offset, store->seen + 1,
2331+
store->offset_alloc);
23302332

2331-
store.offset[store.seen] = cf->do_ftell(cf);
2332-
store.seen++;
2333+
store->offset[store->seen] = cf->do_ftell(cf);
2334+
store->seen++;
23332335
}
23342336
break;
23352337
case SECTION_SEEN:
23362338
/*
2337-
* What we are looking for is in store.key (both
2339+
* What we are looking for is in store->key (both
23382340
* section and var), and its section part is baselen
23392341
* long. We found key (again, both section and var).
23402342
* We would want to know if this key is in the same
23412343
* section as what we are looking for. We already
23422344
* know we are in the same section as what should
2343-
* hold store.key.
2345+
* hold store->key.
23442346
*/
23452347
ep = strrchr(key, '.');
23462348
section_len = ep - key;
23472349

2348-
if ((section_len != store.baselen) ||
2349-
memcmp(key, store.key, section_len+1)) {
2350-
store.state = SECTION_END_SEEN;
2350+
if ((section_len != store->baselen) ||
2351+
memcmp(key, store->key, section_len+1)) {
2352+
store->state = SECTION_END_SEEN;
23512353
break;
23522354
}
23532355

23542356
/*
23552357
* Do not increment matches: this is no match, but we
23562358
* just made sure we are in the desired section.
23572359
*/
2358-
ALLOC_GROW(store.offset, store.seen + 1,
2359-
store.offset_alloc);
2360-
store.offset[store.seen] = cf->do_ftell(cf);
2360+
ALLOC_GROW(store->offset, store->seen + 1,
2361+
store->offset_alloc);
2362+
store->offset[store->seen] = cf->do_ftell(cf);
23612363
/* fallthru */
23622364
case SECTION_END_SEEN:
23632365
case START:
2364-
if (matches(key, value)) {
2365-
ALLOC_GROW(store.offset, store.seen + 1,
2366-
store.offset_alloc);
2367-
store.offset[store.seen] = cf->do_ftell(cf);
2368-
store.state = KEY_SEEN;
2369-
store.seen++;
2366+
if (matches(key, value, store)) {
2367+
ALLOC_GROW(store->offset, store->seen + 1,
2368+
store->offset_alloc);
2369+
store->offset[store->seen] = cf->do_ftell(cf);
2370+
store->state = KEY_SEEN;
2371+
store->seen++;
23702372
} else {
2371-
if (strrchr(key, '.') - key == store.baselen &&
2372-
!strncmp(key, store.key, store.baselen)) {
2373-
store.state = SECTION_SEEN;
2374-
ALLOC_GROW(store.offset,
2375-
store.seen + 1,
2376-
store.offset_alloc);
2377-
store.offset[store.seen] = cf->do_ftell(cf);
2373+
if (strrchr(key, '.') - key == store->baselen &&
2374+
!strncmp(key, store->key, store->baselen)) {
2375+
store->state = SECTION_SEEN;
2376+
ALLOC_GROW(store->offset,
2377+
store->seen + 1,
2378+
store->offset_alloc);
2379+
store->offset[store->seen] =
2380+
cf->do_ftell(cf);
23782381
}
23792382
}
23802383
}
@@ -2389,31 +2392,33 @@ static int write_error(const char *filename)
23892392
return 4;
23902393
}
23912394

2392-
static struct strbuf store_create_section(const char *key)
2395+
static struct strbuf store_create_section(const char *key,
2396+
const struct config_store_data *store)
23932397
{
23942398
const char *dot;
23952399
int i;
23962400
struct strbuf sb = STRBUF_INIT;
23972401

2398-
dot = memchr(key, '.', store.baselen);
2402+
dot = memchr(key, '.', store->baselen);
23992403
if (dot) {
24002404
strbuf_addf(&sb, "[%.*s \"", (int)(dot - key), key);
2401-
for (i = dot - key + 1; i < store.baselen; i++) {
2405+
for (i = dot - key + 1; i < store->baselen; i++) {
24022406
if (key[i] == '"' || key[i] == '\\')
24032407
strbuf_addch(&sb, '\\');
24042408
strbuf_addch(&sb, key[i]);
24052409
}
24062410
strbuf_addstr(&sb, "\"]\n");
24072411
} else {
2408-
strbuf_addf(&sb, "[%.*s]\n", store.baselen, key);
2412+
strbuf_addf(&sb, "[%.*s]\n", store->baselen, key);
24092413
}
24102414

24112415
return sb;
24122416
}
24132417

2414-
static ssize_t write_section(int fd, const char *key)
2418+
static ssize_t write_section(int fd, const char *key,
2419+
const struct config_store_data *store)
24152420
{
2416-
struct strbuf sb = store_create_section(key);
2421+
struct strbuf sb = store_create_section(key, store);
24172422
ssize_t ret;
24182423

24192424
ret = write_in_full(fd, sb.buf, sb.len);
@@ -2422,11 +2427,12 @@ static ssize_t write_section(int fd, const char *key)
24222427
return ret;
24232428
}
24242429

2425-
static ssize_t write_pair(int fd, const char *key, const char *value)
2430+
static ssize_t write_pair(int fd, const char *key, const char *value,
2431+
const struct config_store_data *store)
24262432
{
24272433
int i;
24282434
ssize_t ret;
2429-
int length = strlen(key + store.baselen + 1);
2435+
int length = strlen(key + store->baselen + 1);
24302436
const char *quote = "";
24312437
struct strbuf sb = STRBUF_INIT;
24322438

@@ -2446,7 +2452,7 @@ static ssize_t write_pair(int fd, const char *key, const char *value)
24462452
quote = "\"";
24472453

24482454
strbuf_addf(&sb, "\t%.*s = %s",
2449-
length, key + store.baselen + 1, quote);
2455+
length, key + store->baselen + 1, quote);
24502456

24512457
for (i = 0; value[i]; i++)
24522458
switch (value[i]) {
@@ -2556,6 +2562,9 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
25562562
char *filename_buf = NULL;
25572563
char *contents = NULL;
25582564
size_t contents_sz;
2565+
struct config_store_data store;
2566+
2567+
memset(&store, 0, sizeof(store));
25592568

25602569
/* parse-key returns negative; flip the sign to feed exit(3) */
25612570
ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
@@ -2598,8 +2607,8 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
25982607
}
25992608

26002609
store.key = (char *)key;
2601-
if (write_section(fd, key) < 0 ||
2602-
write_pair(fd, key, value) < 0)
2610+
if (write_section(fd, key, &store) < 0 ||
2611+
write_pair(fd, key, value, &store) < 0)
26032612
goto write_err_out;
26042613
} else {
26052614
struct stat st;
@@ -2638,7 +2647,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
26382647
* As a side effect, we make sure to transform only a valid
26392648
* existing config file.
26402649
*/
2641-
if (git_config_from_file(store_aux, config_filename, NULL)) {
2650+
if (git_config_from_file(store_aux, config_filename, &store)) {
26422651
error("invalid config file %s", config_filename);
26432652
free(store.key);
26442653
if (store.value_regex != NULL &&
@@ -2722,10 +2731,10 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
27222731
/* write the pair (value == NULL means unset) */
27232732
if (value != NULL) {
27242733
if (store.state == START) {
2725-
if (write_section(fd, key) < 0)
2734+
if (write_section(fd, key, &store) < 0)
27262735
goto write_err_out;
27272736
}
2728-
if (write_pair(fd, key, value) < 0)
2737+
if (write_pair(fd, key, value, &store) < 0)
27292738
goto write_err_out;
27302739
}
27312740

@@ -2849,7 +2858,8 @@ static int section_name_is_ok(const char *name)
28492858

28502859
/* if new_name == NULL, the section is removed instead */
28512860
static int git_config_copy_or_rename_section_in_file(const char *config_filename,
2852-
const char *old_name, const char *new_name, int copy)
2861+
const char *old_name,
2862+
const char *new_name, int copy)
28532863
{
28542864
int ret = 0, remove = 0;
28552865
char *filename_buf = NULL;
@@ -2859,6 +2869,9 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename
28592869
FILE *config_file = NULL;
28602870
struct stat st;
28612871
struct strbuf copystr = STRBUF_INIT;
2872+
struct config_store_data store;
2873+
2874+
memset(&store, 0, sizeof(store));
28622875

28632876
if (new_name && !section_name_is_ok(new_name)) {
28642877
ret = error("invalid section name: %s", new_name);
@@ -2928,7 +2941,7 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename
29282941
}
29292942
store.baselen = strlen(new_name);
29302943
if (!copy) {
2931-
if (write_section(out_fd, new_name) < 0) {
2944+
if (write_section(out_fd, new_name, &store) < 0) {
29322945
ret = write_error(get_lock_file_path(&lock));
29332946
goto out;
29342947
}
@@ -2949,7 +2962,7 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename
29492962
output[0] = '\t';
29502963
}
29512964
} else {
2952-
copystr = store_create_section(new_name);
2965+
copystr = store_create_section(new_name, &store);
29532966
}
29542967
}
29552968
remove = 0;

0 commit comments

Comments
 (0)