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>

Johannes Schindelin committed Apr 9, 2018 at 10:32 UTC fee8572c6ddf6afcfeba023067fea36b835a9df4
1 file changed +66 -53
config.c
+66 -53
@@ -2288,7 +2288,7 @@ void git_die_config(const char *key, const char *err, ...)
2288 * Find all the stuff for git_config_set() below.
2289 */
2290
2291 -static struct {
2291 +struct config_store_data {
2292 int baselen;
2293 char *key;
2294 int do_not_match;
@@ -2298,56 +2298,58 @@ static struct {
2298 unsigned int offset_alloc;
2299 enum { START, SECTION_SEEN, SECTION_END_SEEN, KEY_SEEN } state;
2300 unsigned int seen;
2301 -} store;
2301 +};
2302
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)
2305 {
2305 - if (strcmp(key, store.key))
2306 + if (strcmp(key, store->key))
2307 return 0; /* not ours */
2307 - if (!store.value_regex)
2308 + if (!store->value_regex)
2309 return 1; /* always matches */
2309 - if (store.value_regex == CONFIG_REGEX_NONE)
2310 + if (store->value_regex == CONFIG_REGEX_NONE)
2311 return 0; /* never matches */
2312
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));
2315 }
2316
2317 static int store_aux(const char *key, const char *value, void *cb)
2318 {
2319 const char *ep;
2320 size_t section_len;
2321 + struct config_store_data *store = cb;
2322
2321 - switch (store.state) {
2323 + switch (store->state) {
2324 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) {
2327 warning(_("%s has multiple values"), key);
2328 }
2329
2328 - ALLOC_GROW(store.offset, store.seen + 1,
2329 - store.offset_alloc);
2330 + ALLOC_GROW(store->offset, store->seen + 1,
2331 + store->offset_alloc);
2332
2331 - store.offset[store.seen] = cf->do_ftell(cf);
2332 - store.seen++;
2333 + store->offset[store->seen] = cf->do_ftell(cf);
2334 + store->seen++;
2335 }
2336 break;
2337 case SECTION_SEEN:
2338 /*
2337 - * What we are looking for is in store.key (both
2339 + * What we are looking for is in store->key (both
2340 * section and var), and its section part is baselen
2341 * long. We found key (again, both section and var).
2342 * We would want to know if this key is in the same
2343 * section as what we are looking for. We already
2344 * know we are in the same section as what should
2343 - * hold store.key.
2345 + * hold store->key.
2346 */
2347 ep = strrchr(key, '.');
2348 section_len = ep - key;
2349
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;
2353 break;
2354 }
2355
@@ -2355,26 +2357,27 @@ static int store_aux(const char *key, const char *value, void *cb)
2357 * Do not increment matches: this is no match, but we
2358 * just made sure we are in the desired section.
2359 */
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);
2363 /* fallthru */
2364 case SECTION_END_SEEN:
2365 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++;
2372 } 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);
2381 }
2382 }
2383 }
@@ -2389,31 +2392,33 @@ static int write_error(const char *filename)
2392 return 4;
2393 }
2394
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)
2397 {
2398 const char *dot;
2399 int i;
2400 struct strbuf sb = STRBUF_INIT;
2401
2398 - dot = memchr(key, '.', store.baselen);
2402 + dot = memchr(key, '.', store->baselen);
2403 if (dot) {
2404 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++) {
2406 if (key[i] == '"' || key[i] == '\\')
2407 strbuf_addch(&sb, '\\');
2408 strbuf_addch(&sb, key[i]);
2409 }
2410 strbuf_addstr(&sb, "\"]\n");
2411 } else {
2408 - strbuf_addf(&sb, "[%.*s]\n", store.baselen, key);
2412 + strbuf_addf(&sb, "[%.*s]\n", store->baselen, key);
2413 }
2414
2415 return sb;
2416 }
2417
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)
2420 {
2416 - struct strbuf sb = store_create_section(key);
2421 + struct strbuf sb = store_create_section(key, store);
2422 ssize_t ret;
2423
2424 ret = write_in_full(fd, sb.buf, sb.len);
@@ -2422,11 +2427,12 @@ static ssize_t write_section(int fd, const char *key)
2427 return ret;
2428 }
2429
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)
2432 {
2433 int i;
2434 ssize_t ret;
2429 - int length = strlen(key + store.baselen + 1);
2435 + int length = strlen(key + store->baselen + 1);
2436 const char *quote = "";
2437 struct strbuf sb = STRBUF_INIT;
2438
@@ -2446,7 +2452,7 @@ static ssize_t write_pair(int fd, const char *key, const char *value)
2452 quote = "\"";
2453
2454 strbuf_addf(&sb, "\t%.*s = %s",
2449 - length, key + store.baselen + 1, quote);
2455 + length, key + store->baselen + 1, quote);
2456
2457 for (i = 0; value[i]; i++)
2458 switch (value[i]) {
@@ -2556,6 +2562,9 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
2562 char *filename_buf = NULL;
2563 char *contents = NULL;
2564 size_t contents_sz;
2565 + struct config_store_data store;
2566 +
2567 + memset(&store, 0, sizeof(store));
2568
2569 /* parse-key returns negative; flip the sign to feed exit(3) */
2570 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,
2607 }
2608
2609 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)
2612 goto write_err_out;
2613 } else {
2614 struct stat st;
@@ -2638,7 +2647,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
2647 * As a side effect, we make sure to transform only a valid
2648 * existing config file.
2649 */
2641 - if (git_config_from_file(store_aux, config_filename, NULL)) {
2650 + if (git_config_from_file(store_aux, config_filename, &store)) {
2651 error("invalid config file %s", config_filename);
2652 free(store.key);
2653 if (store.value_regex != NULL &&
@@ -2722,10 +2731,10 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
2731 /* write the pair (value == NULL means unset) */
2732 if (value != NULL) {
2733 if (store.state == START) {
2725 - if (write_section(fd, key) < 0)
2734 + if (write_section(fd, key, &store) < 0)
2735 goto write_err_out;
2736 }
2728 - if (write_pair(fd, key, value) < 0)
2737 + if (write_pair(fd, key, value, &store) < 0)
2738 goto write_err_out;
2739 }
2740
@@ -2849,7 +2858,8 @@ static int section_name_is_ok(const char *name)
2858
2859 /* if new_name == NULL, the section is removed instead */
2860 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)
2863 {
2864 int ret = 0, remove = 0;
2865 char *filename_buf = NULL;
@@ -2859,6 +2869,9 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename
2869 FILE *config_file = NULL;
2870 struct stat st;
2871 struct strbuf copystr = STRBUF_INIT;
2872 + struct config_store_data store;
2873 +
2874 + memset(&store, 0, sizeof(store));
2875
2876 if (new_name && !section_name_is_ok(new_name)) {
2877 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
2941 }
2942 store.baselen = strlen(new_name);
2943 if (!copy) {
2931 - if (write_section(out_fd, new_name) < 0) {
2944 + if (write_section(out_fd, new_name, &store) < 0) {
2945 ret = write_error(get_lock_file_path(&lock));
2946 goto out;
2947 }
@@ -2949,7 +2962,7 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename
2962 output[0] = '\t';
2963 }
2964 } else {
2952 - copystr = store_create_section(new_name);
2965 + copystr = store_create_section(new_name, &store);
2966 }
2967 }
2968 remove = 0;