config_file: move reader into `config_read` only Right now, we have multiple call sites which initialize a `reader` structure. As the structure is only actually used inside of `config_read`, we can instead just move the reader inside of the `config_read` function. Instead, we can just pass in the configuration file into `config_read`, which eases code readability.
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 207 208 209 210
diff --git a/src/config_file.c b/src/config_file.c
index f5770d6..3a1a32d 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -118,7 +118,7 @@ typedef struct {
diskfile_backend *snapshot_from;
} diskfile_readonly_backend;
-static int config_read(git_strmap *values, struct reader *reader, git_config_level_t level, int depth);
+static int config_read(git_strmap *values, struct config_file *file, git_config_level_t level, int depth);
static int config_write(diskfile_backend *cfg, const char *key, const regex_t *preg, const char *value);
static char *escape_value(const char *ptr);
@@ -264,13 +264,6 @@ static int refcounted_strmap_alloc(refcounted_strmap **out)
return error;
}
-static void reader_init(struct reader *reader, struct config_file *file)
-{
- memset(reader, 0, sizeof(*reader));
- git_buf_init(&reader->buffer, 0);
- reader->file = file;
-}
-
static void config_file_clear(struct config_file *file)
{
struct config_file *include;
@@ -290,7 +283,6 @@ static void config_file_clear(struct config_file *file)
static int config_open(git_config_backend *cfg, git_config_level_t level)
{
int res;
- struct reader reader;
diskfile_backend *b = (diskfile_backend *)cfg;
b->level = level;
@@ -298,22 +290,15 @@ static int config_open(git_config_backend *cfg, git_config_level_t level)
if ((res = refcounted_strmap_alloc(&b->header.values)) < 0)
return res;
- reader_init(&reader, &b->file);
-
- res = git_futils_readbuffer_updated(
- &reader.buffer, b->file.path, &b->file.checksum, NULL);
-
/* It's fine if the file doesn't exist */
- if (res == GIT_ENOTFOUND)
+ if (!git_path_exists(b->file.path))
return 0;
- if (res < 0 || (res = config_read(b->header.values->values, &reader, level, 0)) < 0) {
+ if (res < 0 || (res = config_read(b->header.values->values, &b->file, level, 0)) < 0) {
refcounted_strmap_free(b->header.values);
b->header.values = NULL;
}
- git_buf_free(&reader.buffer);
-
return res;
}
@@ -354,7 +339,6 @@ static int config_refresh(git_config_backend *cfg)
diskfile_backend *b = (diskfile_backend *)cfg;
refcounted_strmap *values = NULL, *tmp;
struct config_file *include;
- struct reader reader;
int error, modified;
uint32_t i;
@@ -365,11 +349,6 @@ static int config_refresh(git_config_backend *cfg)
if (!modified)
return 0;
- reader_init(&reader, &b->file);
-
- error = git_futils_readbuffer_updated(
- &reader.buffer, b->file.path, &b->file.checksum, NULL);
-
if ((error = refcounted_strmap_alloc(&values)) < 0)
goto out;
@@ -379,7 +358,7 @@ static int config_refresh(git_config_backend *cfg)
}
git_array_clear(b->file.includes);
- if ((error = config_read(values->values, &reader, b->level, 0)) < 0)
+ if ((error = config_read(values->values, &b->file, b->level, 0)) < 0)
goto out;
if ((error = git_mutex_lock(&b->header.values_mutex)) < 0) {
@@ -395,7 +374,6 @@ static int config_refresh(git_config_backend *cfg)
out:
refcounted_strmap_free(values);
- git_buf_free(&reader.buffer);
return (error == GIT_ENOTFOUND) ? 0 : error;
}
@@ -1627,7 +1605,6 @@ static int read_on_variable(
/* Add or append the new config option */
if (!git__strcmp(var->entry->name, "include.path")) {
struct config_file *include;
- struct reader r;
git_buf path = GIT_BUF_INIT;
char *dir;
@@ -1646,48 +1623,55 @@ static int read_on_variable(
git_array_init(include->includes);
include->path = git_buf_detach(&path);
- git_buf_init(&r.buffer, 0);
- memset(&r, 0, sizeof(r));
- r.file = include;
-
- result = git_futils_readbuffer_updated(
- &r.buffer, include->path, &include->checksum, NULL);
+ result = config_read(parse_data->values, include, parse_data->level, parse_data->depth+1);
- if (result == 0) {
- result = config_read(parse_data->values, &r, parse_data->level, parse_data->depth+1);
- } else if (result == GIT_ENOTFOUND) {
+ if (result == GIT_ENOTFOUND) {
giterr_clear();
result = 0;
}
-
- git_buf_free(&r.buffer);
}
return result;
}
-static int config_read(git_strmap *values, struct reader *reader, git_config_level_t level, int depth)
+static int config_read(git_strmap *values, struct config_file *file, git_config_level_t level, int depth)
{
struct parse_data parse_data;
+ struct reader reader;
+ int error;
if (depth >= MAX_INCLUDE_DEPTH) {
giterr_set(GITERR_CONFIG, "maximum config include depth reached");
return -1;
}
+ git_buf_init(&reader.buffer, 0);
+
+ if ((error = git_futils_readbuffer(&reader.buffer, file->path)) < 0)
+ goto out;
+
+ if ((error = git_hash_buf(&file->checksum, reader.buffer.ptr, reader.buffer.size)) < 0)
+ goto out;
+
/* Initialize the reading position */
- reader->read_ptr = reader->buffer.ptr;
- reader->eof = 0;
+ reader.file = file;
+ reader.line_number = 0;
+ reader.read_ptr = reader.buffer.ptr;
+ reader.eof = 0;
/* If the file is empty, there's nothing for us to do */
- if (*reader->read_ptr == '\0')
- return 0;
+ if (*reader.read_ptr == '\0')
+ goto out;
parse_data.values = values;
parse_data.level = level;
parse_data.depth = depth;
- return config_parse(reader, NULL, read_on_variable, NULL, NULL, &parse_data);
+ error = config_parse(&reader, NULL, read_on_variable, NULL, NULL, &parse_data);
+
+out:
+ git_buf_free(&reader.buffer);
+ return error;
}
static int write_section(git_buf *fbuf, const char *key)
@@ -1923,7 +1907,9 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p
struct reader reader;
struct write_data write_data;
- reader_init(&reader, &cfg->file);
+ memset(&reader, 0, sizeof(reader));
+ git_buf_init(&reader.buffer, 0);
+ reader.file = &cfg->file;
if (cfg->locked) {
result = git_buf_puts(&reader.buffer, git_buf_cstr(&cfg->locked_content));
diff --git a/tests/config/include.c b/tests/config/include.c
index 2ce42bd..bcf8e19 100644
--- a/tests/config/include.c
+++ b/tests/config/include.c
@@ -133,3 +133,16 @@ void test_config_include__removing_include_removes_values(void)
cl_git_mkfile("top-level", "");
cl_git_fail(git_config_get_string_buf(&buf, cfg, "foo.bar"));
}
+
+void test_config_include__rewriting_include_refreshes_values(void)
+{
+ cl_git_mkfile("top-level", "[include]\npath = first\n[include]\npath = second");
+ cl_git_mkfile("first", "[first]\nfoo = bar");
+ cl_git_mkfile("second", "[second]\nfoo = bar");
+
+ cl_git_pass(git_config_open_ondisk(&cfg, "top-level"));
+ cl_git_mkfile("first", "[first]\nother = value");
+ cl_git_fail(git_config_get_string_buf(&buf, cfg, "foo.bar"));
+ cl_git_pass(git_config_get_string_buf(&buf, cfg, "first.other"));
+ cl_assert_equal_s(buf.ptr, "value");
+}