Commit 523032cd24e5b238752394bd1b691a3e28ba321e

Carlos Martín Nieto 2014-03-31T09:58:44

config: refresh before reading a value With the isolation of complex reads, we can now try to refresh the on-disk file before reading a value from it. This changes the semantics a bit, as before we could be sure that a string we got from the configuration was valid until we wrote or refreshed. This is no longer the case, as a read can also invalidate the pointer.

diff --git a/include/git2/sys/config.h b/include/git2/sys/config.h
index 0905889..5d606ed 100644
--- a/include/git2/sys/config.h
+++ b/include/git2/sys/config.h
@@ -57,7 +57,7 @@ struct git_config_backend {
 
 	/* Open means open the file/database and parse if necessary */
 	int (*open)(struct git_config_backend *, git_config_level_t level);
-	int (*get)(const struct git_config_backend *, const char *key, const git_config_entry **entry);
+	int (*get)(struct git_config_backend *, const char *key, const git_config_entry **entry);
 	int (*set)(struct git_config_backend *, const char *key, const char *value);
 	int (*set_multivar)(git_config_backend *cfg, const char *name, const char *regexp, const char *value);
 	int (*del)(struct git_config_backend *, const char *key);
diff --git a/src/config_file.c b/src/config_file.c
index 621a7c3..227ec82 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -89,6 +89,7 @@ struct reader {
 typedef struct {
 	git_config_backend parent;
 	git_strmap *values;
+	int readonly;
 } diskfile_header;
 
 typedef struct {
@@ -448,12 +449,19 @@ out:
 /*
  * Internal function that actually gets the value in string form
  */
-static int config_get(const git_config_backend *cfg, const char *key, const git_config_entry **out)
+static int config_get(git_config_backend *cfg, const char *key, const git_config_entry **out)
 {
 	diskfile_header *h = (diskfile_header *)cfg;
-	git_strmap *values = h->values;
-	khiter_t pos = git_strmap_lookup_index(values, key);
+	git_strmap *values;
+	khiter_t pos;
 	cvar_t *var;
+	int error;
+
+	if (!h->readonly && ((error = config_refresh(cfg)) < 0))
+		return error;
+
+	values = h->values;
+	pos = git_strmap_lookup_index(values, key);
 
 	/* no error message; the config system will write one */
 	if (!git_strmap_valid_index(values, pos))
@@ -783,6 +791,7 @@ int git_config_file__snapshot(git_config_backend **out, diskfile_backend *in)
 
 	backend->snapshot_from = in;
 
+	backend->header.readonly = 1;
 	backend->header.parent.version = GIT_CONFIG_BACKEND_VERSION;
 	backend->header.parent.open = config_readonly_open;
 	backend->header.parent.get = config_get;
diff --git a/tests/config/refresh.c b/tests/config/refresh.c
index 99d677f..08cd45b 100644
--- a/tests/config/refresh.c
+++ b/tests/config/refresh.c
@@ -26,9 +26,6 @@ void test_config_refresh__update_value(void)
 
 	cl_git_rewritefile(TEST_FILE, "[section]\n\tvalue = 10\n\n");
 
-	cl_git_pass(git_config_get_int32(&v, cfg, "section.value"));
-	cl_assert_equal_i(1, v);
-
 	cl_git_pass(git_config_refresh(cfg));
 
 	cl_git_pass(git_config_get_int32(&v, cfg, "section.value"));
@@ -53,9 +50,9 @@ void test_config_refresh__delete_value(void)
 
 	cl_git_rewritefile(TEST_FILE, "[section]\n\tnewval = 10\n\n");
 
-	cl_git_pass(git_config_get_int32(&v, cfg, "section.value"));
-	cl_assert_equal_i(1, v);
-	cl_git_fail(git_config_get_int32(&v, cfg, "section.newval"));
+	cl_git_fail_with(GIT_ENOTFOUND, git_config_get_int32(&v, cfg, "section.value"));
+
+	cl_git_pass(git_config_get_int32(&v, cfg, "section.newval"));
 
 	cl_git_pass(git_config_refresh(cfg));
 
diff --git a/tests/config/snapshot.c b/tests/config/snapshot.c
index a9d3ead..c9f1592 100644
--- a/tests/config/snapshot.c
+++ b/tests/config/snapshot.c
@@ -19,11 +19,6 @@ void test_config_snapshot__create_snapshot(void)
 	cl_git_mkfile(filename, "[old]\nvalue = 56\n");
 
 	cl_git_pass(git_config_get_int32(&tmp, cfg, "old.value"));
-	cl_assert_equal_i(5, tmp);
-
-	cl_git_pass(git_config_refresh(cfg));
-
-	cl_git_pass(git_config_get_int32(&tmp, cfg, "old.value"));
 	cl_assert_equal_i(56, tmp);
 
 	cl_git_pass(git_config_get_int32(&tmp, snapshot, "old.value"));