Merge pull request #3491 from libgit2/cmn/config-checksum Use checksums to detect config file changes
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 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206
diff --git a/src/config_file.c b/src/config_file.c
index 46f21c0..5f5e309 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -77,8 +77,7 @@ typedef struct git_config_file_iter {
(iter) = (tmp))
struct reader {
- time_t file_mtime;
- size_t file_size;
+ git_oid checksum;
char *file_path;
git_buf buffer;
char *read_ptr;
@@ -285,7 +284,7 @@ static int config_open(git_config_backend *cfg, git_config_level_t level)
git_buf_init(&reader->buffer, 0);
res = git_futils_readbuffer_updated(
- &reader->buffer, b->file_path, &reader->file_mtime, &reader->file_size, NULL);
+ &reader->buffer, b->file_path, &reader->checksum, NULL);
/* It's fine if the file doesn't exist */
if (res == GIT_ENOTFOUND)
@@ -345,7 +344,7 @@ static int config_refresh(git_config_backend *cfg)
reader = git_array_get(b->readers, i);
error = git_futils_readbuffer_updated(
&reader->buffer, reader->file_path,
- &reader->file_mtime, &reader->file_size, &updated);
+ &reader->checksum, &updated);
if (error < 0 && error != GIT_ENOTFOUND)
return error;
@@ -1618,7 +1617,7 @@ static int read_on_variable(
git_buf_init(&r->buffer, 0);
result = git_futils_readbuffer_updated(
- &r->buffer, r->file_path, &r->file_mtime, &r->file_size, NULL);
+ &r->buffer, r->file_path, &r->checksum, NULL);
if (result == 0) {
result = config_read(parse_data->values, parse_data->cfg_file, r, parse_data->level, parse_data->depth+1);
@@ -1894,7 +1893,7 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p
} else {
/* Lock the file */
if ((result = git_filebuf_open(
- &file, cfg->file_path, 0, GIT_CONFIG_FILE_MODE)) < 0) {
+ &file, cfg->file_path, GIT_FILEBUF_HASH_CONTENTS, GIT_CONFIG_FILE_MODE)) < 0) {
git_buf_free(&reader->buffer);
return result;
}
@@ -1945,10 +1944,6 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p
git_buf_attach(&cfg->locked_content, git_buf_detach(&buf), len);
} else {
git_filebuf_write(&file, git_buf_cstr(&buf), git_buf_len(&buf));
-
- /* refresh stats - if this errors, then commit will error too */
- (void)git_filebuf_stats(&reader->file_mtime, &reader->file_size, &file);
-
result = git_filebuf_commit(&file);
}
diff --git a/src/fileops.c b/src/fileops.c
index 57d2ce9..cfc22bb 100644
--- a/src/fileops.c
+++ b/src/fileops.c
@@ -153,13 +153,15 @@ int git_futils_readbuffer_fd(git_buf *buf, git_file fd, size_t len)
}
int git_futils_readbuffer_updated(
- git_buf *buf, const char *path, time_t *mtime, size_t *size, int *updated)
+ git_buf *out, const char *path, git_oid *checksum, int *updated)
{
+ int error;
git_file fd;
struct stat st;
- bool changed = false;
+ git_buf buf = GIT_BUF_INIT;
+ git_oid checksum_new;
- assert(buf && path && *path);
+ assert(out && path && *path);
if (updated != NULL)
*updated = 0;
@@ -178,45 +180,50 @@ int git_futils_readbuffer_updated(
return -1;
}
- /*
- * If we were given a time and/or a size, we only want to read the file
- * if it has been modified.
- */
- if (size && *size != (size_t)st.st_size)
- changed = true;
- if (mtime && *mtime != (time_t)st.st_mtime)
- changed = true;
- if (!size && !mtime)
- changed = true;
-
- if (!changed) {
- return 0;
- }
-
- if (mtime != NULL)
- *mtime = st.st_mtime;
- if (size != NULL)
- *size = (size_t)st.st_size;
-
if ((fd = git_futils_open_ro(path)) < 0)
return fd;
- if (git_futils_readbuffer_fd(buf, fd, (size_t)st.st_size) < 0) {
+ if (git_futils_readbuffer_fd(&buf, fd, (size_t)st.st_size) < 0) {
p_close(fd);
return -1;
}
p_close(fd);
+ if ((error = git_hash_buf(&checksum_new, buf.ptr, buf.size)) < 0) {
+ git_buf_free(&buf);
+ return error;
+ }
+
+ /*
+ * If we were given a checksum, we only want to use it if it's different
+ */
+ if (checksum && !git_oid__cmp(checksum, &checksum_new)) {
+ git_buf_free(&buf);
+ if (updated)
+ *updated = 0;
+
+ return 0;
+ }
+
+ /*
+ * If we're here, the file did change, or the user didn't have an old version
+ */
+ if (checksum)
+ git_oid_cpy(checksum, &checksum_new);
+
if (updated != NULL)
*updated = 1;
+ git_buf_swap(out, &buf);
+ git_buf_free(&buf);
+
return 0;
}
int git_futils_readbuffer(git_buf *buf, const char *path)
{
- return git_futils_readbuffer_updated(buf, path, NULL, NULL, NULL);
+ return git_futils_readbuffer_updated(buf, path, NULL, NULL);
}
int git_futils_writebuffer(
diff --git a/src/fileops.h b/src/fileops.h
index 572ff01..c6db195 100644
--- a/src/fileops.h
+++ b/src/fileops.h
@@ -13,6 +13,7 @@
#include "path.h"
#include "pool.h"
#include "strmap.h"
+#include "oid.h"
/**
* Filebuffer methods
@@ -21,7 +22,7 @@
*/
extern int git_futils_readbuffer(git_buf *obj, const char *path);
extern int git_futils_readbuffer_updated(
- git_buf *obj, const char *path, time_t *mtime, size_t *size, int *updated);
+ git_buf *obj, const char *path, git_oid *checksum, int *updated);
extern int git_futils_readbuffer_fd(git_buf *obj, git_file fd, size_t len);
extern int git_futils_writebuffer(
diff --git a/tests/config/stress.c b/tests/config/stress.c
index 503f44f..6e96042 100644
--- a/tests/config/stress.c
+++ b/tests/config/stress.c
@@ -107,3 +107,23 @@ void test_config_stress__complex(void)
git_config_free(config);
}
+
+void test_config_stress__quick_write(void)
+{
+ git_config *config_w, *config_r;
+ const char *path = "./config-quick-write";
+ const char *key = "quick.write";
+ int32_t i;
+
+ /* Create an external writer for one instance with the other one */
+ cl_git_pass(git_config_open_ondisk(&config_w, path));
+ cl_git_pass(git_config_open_ondisk(&config_r, path));
+
+ /* Write and read in the same second (repeat to increase the chance of it happening) */
+ for (i = 0; i < 10; i++) {
+ int32_t val;
+ cl_git_pass(git_config_set_int32(config_w, key, i));
+ cl_git_pass(git_config_get_int32(&val, config_r, key));
+ cl_assert_equal_i(i, val);
+ }
+}