git_config_set: do not use a state machine

While a neat theoretical construct, state machines are hard to read. In this instance, it does not even make a whole lot of sense because we are more interested in flags, anyway: has the section been seen? Has the key been seen? Does the current section match the key we are looking for? Besides, the state `SECTION_SEEN` was named in a misleading way: it did not indicate that we saw the section matching the key we are looking for, but it instead indicated that we are *currently* in that section. Let's just replace the state machine logic by clear and obvious flags. This will also make it easier to review the upcoming patches to use the newly-introduced `event_fn` callback of the config parser. 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 5221c3159f670281ff36b6adbdf568661e930b50
1 file changed +29 -30
config.c
+29 -30
@@ -2296,7 +2296,7 @@ struct config_store_data {
2296 int multi_replace;
2297 size_t *seen;
2298 unsigned int seen_nr, seen_alloc;
2299 - enum { START, SECTION_SEEN, SECTION_END_SEEN, KEY_SEEN } state;
2299 + unsigned int key_seen:1, section_seen:1, is_keys_section:1;
2300 };
2301
2302 static int matches(const char *key, const char *value,
@@ -2319,8 +2319,7 @@ static int store_aux(const char *key, const char *value, void *cb)
2319 size_t section_len;
2320 struct config_store_data *store = cb;
2321
2322 - switch (store->state) {
2323 - case KEY_SEEN:
2322 + if (store->key_seen) {
2323 if (matches(key, value, store)) {
2324 if (store->seen_nr == 1 && store->multi_replace == 0) {
2325 warning(_("%s has multiple values"), key);
@@ -2332,8 +2331,8 @@ static int store_aux(const char *key, const char *value, void *cb)
2331 store->seen[store->seen_nr] = cf->do_ftell(cf);
2332 store->seen_nr++;
2333 }
2335 - break;
2336 - case SECTION_SEEN:
2334 + return 0;
2335 + } else if (store->is_keys_section) {
2336 /*
2337 * What we are looking for is in store->key (both
2338 * section and var), and its section part is baselen
@@ -2348,10 +2347,9 @@ static int store_aux(const char *key, const char *value, void *cb)
2347
2348 if ((section_len != store->baselen) ||
2349 memcmp(key, store->key, section_len+1)) {
2351 - store->state = SECTION_END_SEEN;
2352 - break;
2350 + store->is_keys_section = 0;
2351 + return 0;
2352 }
2354 -
2353 /*
2354 * Do not increment matches: this is no match, but we
2355 * just made sure we are in the desired section.
@@ -2359,27 +2357,29 @@ static int store_aux(const char *key, const char *value, void *cb)
2357 ALLOC_GROW(store->seen, store->seen_nr + 1,
2358 store->seen_alloc);
2359 store->seen[store->seen_nr] = cf->do_ftell(cf);
2362 - /* fallthru */
2363 - case SECTION_END_SEEN:
2364 - case START:
2365 - if (matches(key, value, store)) {
2366 - ALLOC_GROW(store->seen, store->seen_nr + 1,
2367 - store->seen_alloc);
2368 - store->seen[store->seen_nr] = cf->do_ftell(cf);
2369 - store->state = KEY_SEEN;
2370 - store->seen_nr++;
2371 - } else {
2372 - if (strrchr(key, '.') - key == store->baselen &&
2373 - !strncmp(key, store->key, store->baselen)) {
2374 - store->state = SECTION_SEEN;
2375 - ALLOC_GROW(store->seen,
2376 - store->seen_nr + 1,
2377 - store->seen_alloc);
2378 - store->seen[store->seen_nr] =
2379 - cf->do_ftell(cf);
2380 - }
2360 + }
2361 +
2362 + if (matches(key, value, store)) {
2363 + ALLOC_GROW(store->seen, store->seen_nr + 1,
2364 + store->seen_alloc);
2365 + store->seen[store->seen_nr] = cf->do_ftell(cf);
2366 + store->seen_nr++;
2367 + store->key_seen = 1;
2368 + store->section_seen = 1;
2369 + store->is_keys_section = 1;
2370 + } else {
2371 + if (strrchr(key, '.') - key == store->baselen &&
2372 + !strncmp(key, store->key, store->baselen)) {
2373 + store->section_seen = 1;
2374 + store->is_keys_section = 1;
2375 + ALLOC_GROW(store->seen,
2376 + store->seen_nr + 1,
2377 + store->seen_alloc);
2378 + store->seen[store->seen_nr] =
2379 + cf->do_ftell(cf);
2380 }
2381 }
2382 +
2383 return 0;
2384 }
2385
@@ -2637,7 +2637,6 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
2637
2638 ALLOC_GROW(store.seen, 1, store.seen_alloc);
2639 store.seen[0] = 0;
2640 - store.state = START;
2640 store.seen_nr = 0;
2641
2642 /*
@@ -2705,7 +2704,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
2704 new_line = 0;
2705 if (store.seen[i] == 0) {
2706 store.seen[i] = copy_end = contents_sz;
2708 - } else if (store.state != KEY_SEEN) {
2707 + } else if (!store.key_seen) {
2708 copy_end = store.seen[i];
2709 } else
2710 copy_end = find_beginning_of_line(
@@ -2729,7 +2728,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
2728
2729 /* write the pair (value == NULL means unset) */
2730 if (value != NULL) {
2732 - if (store.state == START) {
2731 + if (!store.section_seen) {
2732 if (write_section(fd, key, &store) < 0)
2733 goto write_err_out;
2734 }