Commit 3e1c137ab21c4cb9bcebf382bd5dd16f7f8bb685
2019-06-27T08:24:21
config_file: move refresh into `write` function We are quite lazy in how we refresh our config file backend when updating any of its keys: instead of just updating our in-memory representation of the keys, we just discard the old set of keys and then re-read the config file contents from disk. This refresh currently happens separately at every callsite of `config_write`, but it is clear that we _always_ want to refresh if we have written the config file to disk. If we didn't, then we'd run around with an outdated config file backend that does not represent what we have on disk. By moving the refresh into `config_write`, we are also able to optimize the case where the config file is currently locked. Before, we would've tried to re-read the file even if we have only updated its cached contents without touching the on-disk file. Thus we'd have unnecessarily stat'd the file, even though we know that it shouldn't have been modified in the meantime due to its lock.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56
diff --git a/src/config_file.c b/src/config_file.c
index d238cb7..91fce1c 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -284,8 +284,6 @@ static int config_set(git_config_backend *cfg, const char *name, const char *val
if ((error = config_write(b, name, key, NULL, esc_value)) < 0)
goto out;
- error = config_refresh(cfg);
-
out:
git_config_entries_free(entries);
git__free(esc_value);
@@ -352,8 +350,6 @@ static int config_set_multivar(
if ((result = config_write(b, name, key, &preg, value)) < 0)
goto out;
- result = config_refresh(cfg);
-
out:
git__free(key);
p_regfree(&preg);
@@ -385,9 +381,6 @@ static int config_delete(git_config_backend *cfg, const char *name)
if ((error = config_write(b, name, entry->name, NULL, NULL)) < 0)
goto out;
- if ((error = config_refresh(cfg)) < 0)
- goto out;
-
out:
git_config_entries_free(entries);
git__free(key);
@@ -426,9 +419,6 @@ static int config_delete_multivar(git_config_backend *cfg, const char *name, con
if ((result = config_write(b, name, key, &preg, NULL)) < 0)
goto out;
- if ((result = config_refresh(cfg)) < 0)
- goto out;
-
out:
git_config_entries_free(entries);
git__free(key);
@@ -1208,7 +1198,12 @@ static int config_write(diskfile_backend *cfg, const char *orig_key, const char
git_buf_attach(&cfg->locked_content, git_buf_detach(&buf), len);
} else {
git_filebuf_write(&file, git_buf_cstr(&buf), git_buf_len(&buf));
- result = git_filebuf_commit(&file);
+
+ if ((result = git_filebuf_commit(&file)) < 0)
+ goto done;
+
+ if ((result = config_refresh(&cfg->header.parent)) < 0)
+ goto done;
}
done: