diff options
author | Patrick Steinhardt <ps@pks.im> | 2019-11-05 13:04:10 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-11-05 13:04:10 +0100 |
commit | 5d773a1833ef6d0fb2093e00b1cf9bfb668a1ffc (patch) | |
tree | b8dc3925c4d3b4b4c1d652c8e5d371371044c2af | |
parent | 82d7a114eb56c64da15dc278c90385dd3f83c28c (diff) | |
parent | 56b203a5e0a5348700d85479b0070289d37c2cf0 (diff) | |
download | libgit2-5d773a1833ef6d0fb2093e00b1cf9bfb668a1ffc.tar.gz |
Merge pull request #5282 from pks-t/pks/config-file-iterator-race
config_file: fix race when creating an iterator
-rw-r--r-- | src/config_file.c | 155 | ||||
-rw-r--r-- | src/config_snapshot.c | 44 |
2 files changed, 99 insertions, 100 deletions
diff --git a/src/config_file.c b/src/config_file.c index bf770c95b..c9e36493e 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -9,23 +9,17 @@ #include "git2/config.h" #include "git2/sys/config.h" -#include "git2/types.h" #include "array.h" -#include "buf_text.h" #include "buffer.h" #include "config_backend.h" #include "config_entries.h" #include "config_parse.h" #include "filebuf.h" #include "regexp.h" -#include "strmap.h" #include "sysdir.h" #include "wildmatch.h" -#include <ctype.h> -#include <sys/types.h> - /* Max depth for [include] directives */ #define MAX_INCLUDE_DEPTH 10 @@ -60,9 +54,9 @@ typedef struct { unsigned int depth; } config_file_parse_data; -static int config_read(git_config_entries *entries, const git_repository *repo, config_file *file, git_config_level_t level, int depth); -static int config_read_buffer(git_config_entries *entries, const git_repository *repo, config_file *file, git_config_level_t level, int depth, const char *buf, size_t buflen); -static int config_write(config_file_backend *cfg, const char *orig_key, const char *key, const git_regexp *preg, const char *value); +static int config_file_read(git_config_entries *entries, const git_repository *repo, config_file *file, git_config_level_t level, int depth); +static int config_file_read_buffer(git_config_entries *entries, const git_repository *repo, config_file *file, git_config_level_t level, int depth, const char *buf, size_t buflen); +static int config_file_write(config_file_backend *cfg, const char *orig_key, const char *key, const git_regexp *preg, const char *value); static char *escape_value(const char *ptr); /** @@ -70,21 +64,21 @@ static char *escape_value(const char *ptr); * refcount. This is its own function to make sure we use the mutex to * avoid the map pointer from changing under us. */ -static git_config_entries *diskfile_entries_take(config_file_backend *b) +static int config_file_entries_take(git_config_entries **out, config_file_backend *b) { - git_config_entries *entries; + int error; - if (git_mutex_lock(&b->values_mutex) < 0) { - git_error_set(GIT_ERROR_OS, "failed to lock config backend"); - return NULL; + if ((error = git_mutex_lock(&b->values_mutex)) < 0) { + git_error_set(GIT_ERROR_OS, "failed to lock config backend"); + return error; } - entries = b->entries; - git_config_entries_incref(entries); + git_config_entries_incref(b->entries); + *out = b->entries; git_mutex_unlock(&b->values_mutex); - return entries; + return 0; } static void config_file_clear(config_file *file) @@ -103,7 +97,7 @@ static void config_file_clear(config_file *file) git__free(file->path); } -static int config_open(git_config_backend *cfg, git_config_level_t level, const git_repository *repo) +static int config_file_open(git_config_backend *cfg, git_config_level_t level, const git_repository *repo) { config_file_backend *b = GIT_CONTAINER_OF(cfg, config_file_backend, parent); int res; @@ -117,7 +111,7 @@ static int config_open(git_config_backend *cfg, git_config_level_t level, const if (!git_path_exists(b->file.path)) return 0; - if (res < 0 || (res = config_read(b->entries, repo, &b->file, level, 0)) < 0) { + if (res < 0 || (res = config_file_read(b->entries, repo, &b->file, level, 0)) < 0) { git_config_entries_free(b->entries); b->entries = NULL; } @@ -125,7 +119,7 @@ static int config_open(git_config_backend *cfg, git_config_level_t level, const return res; } -static int config_is_modified(int *modified, config_file *file) +static int config_file_is_modified(int *modified, config_file *file) { config_file *include; git_buf buf = GIT_BUF_INIT; @@ -151,7 +145,7 @@ static int config_is_modified(int *modified, config_file *file) check_includes: git_array_foreach(file->includes, i, include) { - if ((error = config_is_modified(modified, include)) < 0 || *modified) + if ((error = config_file_is_modified(modified, include)) < 0 || *modified) goto out; } @@ -161,7 +155,7 @@ out: return error; } -static int config_set_entries(git_config_backend *cfg, git_config_entries *entries) +static int config_file_set_entries(git_config_backend *cfg, git_config_entries *entries) { config_file_backend *b = GIT_CONTAINER_OF(cfg, config_file_backend, parent); git_config_entries *old = NULL; @@ -193,16 +187,16 @@ out: return error; } -static int config_refresh_from_buffer(git_config_backend *cfg, const char *buf, size_t buflen) +static int config_file_refresh_from_buffer(git_config_backend *cfg, const char *buf, size_t buflen) { config_file_backend *b = GIT_CONTAINER_OF(cfg, config_file_backend, parent); git_config_entries *entries = NULL; int error; if ((error = git_config_entries_new(&entries)) < 0 || - (error = config_read_buffer(entries, b->repo, &b->file, - b->level, 0, buf, buflen)) < 0 || - (error = config_set_entries(cfg, entries)) < 0) + (error = config_file_read_buffer(entries, b->repo, &b->file, + b->level, 0, buf, buflen)) < 0 || + (error = config_file_set_entries(cfg, entries)) < 0) goto out; entries = NULL; @@ -211,7 +205,7 @@ out: return error; } -static int config_refresh(git_config_backend *cfg) +static int config_file_refresh(git_config_backend *cfg) { config_file_backend *b = GIT_CONTAINER_OF(cfg, config_file_backend, parent); git_config_entries *entries = NULL; @@ -220,15 +214,15 @@ static int config_refresh(git_config_backend *cfg) if (cfg->readonly) return 0; - if ((error = config_is_modified(&modified, &b->file)) < 0 && error != GIT_ENOTFOUND) + if ((error = config_file_is_modified(&modified, &b->file)) < 0 && error != GIT_ENOTFOUND) goto out; if (!modified) return 0; if ((error = git_config_entries_new(&entries)) < 0 || - (error = config_read(entries, b->repo, &b->file, b->level, 0)) < 0 || - (error = config_set_entries(cfg, entries)) < 0) + (error = config_file_read(entries, b->repo, &b->file, b->level, 0)) < 0 || + (error = config_file_set_entries(cfg, entries)) < 0) goto out; entries = NULL; @@ -238,7 +232,7 @@ out: return (error == GIT_ENOTFOUND) ? 0 : error; } -static void backend_free(git_config_backend *_backend) +static void config_file_free(git_config_backend *_backend) { config_file_backend *backend = GIT_CONTAINER_OF(_backend, config_file_backend, parent); @@ -251,26 +245,33 @@ static void backend_free(git_config_backend *_backend) git__free(backend); } -static int config_iterator_new( +static int config_file_iterator( git_config_iterator **iter, struct git_config_backend *backend) { config_file_backend *b = GIT_CONTAINER_OF(backend, config_file_backend, parent); - git_config_entries *entries = NULL; + git_config_entries *dupped = NULL, *entries = NULL; int error; - if ((error = config_refresh(backend)) < 0 || - (error = git_config_entries_dup(&entries, b->entries)) < 0 || - (error = git_config_entries_iterator_new(iter, entries)) < 0) + if ((error = config_file_refresh(backend)) < 0 || + (error = config_file_entries_take(&entries, b)) < 0 || + (error = git_config_entries_dup(&dupped, entries)) < 0 || + (error = git_config_entries_iterator_new(iter, dupped)) < 0) goto out; out: /* Let iterator delete duplicated entries when it's done */ git_config_entries_free(entries); + git_config_entries_free(dupped); return error; } -static int config_set(git_config_backend *cfg, const char *name, const char *value) +static int config_file_snapshot(git_config_backend **out, git_config_backend *backend) +{ + return git_config_backend_snapshot(out, backend); +} + +static int config_file_set(git_config_backend *cfg, const char *name, const char *value) { config_file_backend *b = GIT_CONTAINER_OF(cfg, config_file_backend, parent); git_config_entries *entries; @@ -281,8 +282,8 @@ static int config_set(git_config_backend *cfg, const char *name, const char *val if ((error = git_config__normalize_name(name, &key)) < 0) return error; - if ((entries = diskfile_entries_take(b)) == NULL) - return -1; + if ((error = config_file_entries_take(&entries, b)) < 0) + return error; /* Check whether we'd be modifying an included or multivar key */ if ((error = git_config_entries_get_unique(&existing, entries, key)) < 0) { @@ -302,7 +303,7 @@ static int config_set(git_config_backend *cfg, const char *name, const char *val GIT_ERROR_CHECK_ALLOC(esc_value); } - if ((error = config_write(b, name, key, NULL, esc_value)) < 0) + if ((error = config_file_write(b, name, key, NULL, esc_value)) < 0) goto out; out: @@ -313,7 +314,7 @@ out: } /* release the map containing the entry as an equivalent to freeing it */ -static void free_diskfile_entry(git_config_entry *entry) +static void config_file_entry_free(git_config_entry *entry) { git_config_entries *entries = (git_config_entries *) entry->payload; git_config_entries_free(entries); @@ -322,32 +323,32 @@ static void free_diskfile_entry(git_config_entry *entry) /* * Internal function that actually gets the value in string form */ -static int config_get(git_config_backend *cfg, const char *key, git_config_entry **out) +static int config_file_get(git_config_backend *cfg, const char *key, git_config_entry **out) { config_file_backend *h = GIT_CONTAINER_OF(cfg, config_file_backend, parent); git_config_entries *entries = NULL; git_config_entry *entry; int error = 0; - if (!h->parent.readonly && ((error = config_refresh(cfg)) < 0)) + if (!h->parent.readonly && ((error = config_file_refresh(cfg)) < 0)) return error; - if ((entries = diskfile_entries_take(h)) == NULL) - return -1; + if ((error = config_file_entries_take(&entries, h)) < 0) + return error; if ((error = (git_config_entries_get(&entry, entries, key))) < 0) { git_config_entries_free(entries); return error; } - entry->free = free_diskfile_entry; + entry->free = config_file_entry_free; entry->payload = entries; *out = entry; return 0; } -static int config_set_multivar( +static int config_file_set_multivar( git_config_backend *cfg, const char *name, const char *regexp, const char *value) { config_file_backend *b = GIT_CONTAINER_OF(cfg, config_file_backend, parent); @@ -363,8 +364,8 @@ static int config_set_multivar( if ((result = git_regexp_compile(&preg, regexp, 0)) < 0) goto out; - /* If we do have it, set call config_write() and reload */ - if ((result = config_write(b, name, key, &preg, value)) < 0) + /* If we do have it, set call config_file_write() and reload */ + if ((result = config_file_write(b, name, key, &preg, value)) < 0) goto out; out: @@ -374,7 +375,7 @@ out: return result; } -static int config_delete(git_config_backend *cfg, const char *name) +static int config_file_delete(git_config_backend *cfg, const char *name) { config_file_backend *b = GIT_CONTAINER_OF(cfg, config_file_backend, parent); git_config_entries *entries = NULL; @@ -385,7 +386,7 @@ static int config_delete(git_config_backend *cfg, const char *name) if ((error = git_config__normalize_name(name, &key)) < 0) goto out; - if ((entries = diskfile_entries_take(b)) == NULL) + if ((error = config_file_entries_take(&entries, b)) < 0) goto out; /* Check whether we'd be modifying an included or multivar key */ @@ -395,7 +396,7 @@ static int config_delete(git_config_backend *cfg, const char *name) goto out; } - if ((error = config_write(b, name, entry->name, NULL, NULL)) < 0) + if ((error = config_file_write(b, name, entry->name, NULL, NULL)) < 0) goto out; out: @@ -404,7 +405,7 @@ out: return error; } -static int config_delete_multivar(git_config_backend *cfg, const char *name, const char *regexp) +static int config_file_delete_multivar(git_config_backend *cfg, const char *name, const char *regexp) { config_file_backend *b = GIT_CONTAINER_OF(cfg, config_file_backend, parent); git_config_entries *entries = NULL; @@ -416,10 +417,8 @@ static int config_delete_multivar(git_config_backend *cfg, const char *name, con if ((result = git_config__normalize_name(name, &key)) < 0) goto out; - if ((entries = diskfile_entries_take(b)) == NULL) { - result = -1; + if ((result = config_file_entries_take(&entries, b)) < 0) goto out; - } if ((result = git_config_entries_get(&entry, entries, key)) < 0) { if (result == GIT_ENOTFOUND) @@ -430,7 +429,7 @@ static int config_delete_multivar(git_config_backend *cfg, const char *name, con if ((result = git_regexp_compile(&preg, regexp, 0)) < 0) goto out; - if ((result = config_write(b, name, key, &preg, NULL)) < 0) + if ((result = config_file_write(b, name, key, &preg, NULL)) < 0) goto out; out: @@ -440,7 +439,7 @@ out: return result; } -static int config_lock(git_config_backend *_cfg) +static int config_file_lock(git_config_backend *_cfg) { config_file_backend *cfg = GIT_CONTAINER_OF(_cfg, config_file_backend, parent); int error; @@ -459,7 +458,7 @@ static int config_lock(git_config_backend *_cfg) } -static int config_unlock(git_config_backend *_cfg, int success) +static int config_file_unlock(git_config_backend *_cfg, int success) { config_file_backend *cfg = GIT_CONTAINER_OF(_cfg, config_file_backend, parent); int error = 0; @@ -490,17 +489,17 @@ int git_config_backend_from_file(git_config_backend **out, const char *path) GIT_ERROR_CHECK_ALLOC(backend->file.path); git_array_init(backend->file.includes); - backend->parent.open = config_open; - backend->parent.get = config_get; - backend->parent.set = config_set; - backend->parent.set_multivar = config_set_multivar; - backend->parent.del = config_delete; - backend->parent.del_multivar = config_delete_multivar; - backend->parent.iterator = config_iterator_new; - backend->parent.snapshot = git_config_backend_snapshot; - backend->parent.lock = config_lock; - backend->parent.unlock = config_unlock; - backend->parent.free = backend_free; + backend->parent.open = config_file_open; + backend->parent.get = config_file_get; + backend->parent.set = config_file_set; + backend->parent.set_multivar = config_file_set_multivar; + backend->parent.del = config_file_delete; + backend->parent.del_multivar = config_file_delete_multivar; + backend->parent.iterator = config_file_iterator; + backend->parent.snapshot = config_file_snapshot; + backend->parent.lock = config_file_lock; + backend->parent.unlock = config_file_unlock; + backend->parent.free = config_file_free; *out = (git_config_backend *)backend; @@ -574,8 +573,8 @@ static int parse_include(config_file_parse_data *parse_data, const char *file) git_array_init(include->includes); include->path = git_buf_detach(&path); - result = config_read(parse_data->entries, parse_data->repo, - include, parse_data->level, parse_data->depth+1); + result = config_file_read(parse_data->entries, parse_data->repo, include, + parse_data->level, parse_data->depth+1); if (result == GIT_ENOTFOUND) { git_error_clear(); @@ -793,7 +792,7 @@ static int read_on_variable( return result; } -static int config_read_buffer( +static int config_file_read_buffer( git_config_entries *entries, const git_repository *repo, config_file *file, @@ -833,7 +832,7 @@ out: return error; } -static int config_read( +static int config_file_read( git_config_entries *entries, const git_repository *repo, config_file *file, @@ -856,8 +855,8 @@ static int config_read( if ((error = git_hash_buf(&file->checksum, contents.ptr, contents.size)) < 0) goto out; - if ((error = config_read_buffer(entries, repo, file, level, depth, - contents.ptr, contents.size)) < 0) + if ((error = config_file_read_buffer(entries, repo, file, level, depth, + contents.ptr, contents.size)) < 0) goto out; out: @@ -1088,7 +1087,7 @@ static int write_on_eof( /* * This is pretty much the parsing, except we write out anything we don't have */ -static int config_write(config_file_backend *cfg, const char *orig_key, const char *key, const git_regexp *preg, const char* value) +static int config_file_write(config_file_backend *cfg, const char *orig_key, const char *key, const git_regexp *preg, const char* value) { char *orig_section = NULL, *section = NULL, *orig_name, *name, *ldot; @@ -1149,7 +1148,7 @@ static int config_write(config_file_backend *cfg, const char *orig_key, const ch if ((error = git_filebuf_commit(&file)) < 0) goto done; - if ((error = config_refresh_from_buffer(&cfg->parent, buf.ptr, buf.size)) < 0) + if ((error = config_file_refresh_from_buffer(&cfg->parent, buf.ptr, buf.size)) < 0) goto done; } diff --git a/src/config_snapshot.c b/src/config_snapshot.c index e89a13886..62b9068fb 100644 --- a/src/config_snapshot.c +++ b/src/config_snapshot.c @@ -22,7 +22,7 @@ static int config_error_readonly(void) return -1; } -static int config_iterator_new_readonly( +static int config_snapshot_iterator( git_config_iterator **iter, struct git_config_backend *backend) { @@ -41,13 +41,13 @@ out: } /* release the map containing the entry as an equivalent to freeing it */ -static void free_diskfile_entry(git_config_entry *entry) +static void config_snapshot_entry_free(git_config_entry *entry) { git_config_entries *entries = (git_config_entries *) entry->payload; git_config_entries_free(entries); } -static int config_get_readonly(git_config_backend *cfg, const char *key, git_config_entry **out) +static int config_snapshot_get(git_config_backend *cfg, const char *key, git_config_entry **out) { config_snapshot_backend *b = GIT_CONTAINER_OF(cfg, config_snapshot_backend, parent); git_config_entries *entries = NULL; @@ -68,14 +68,14 @@ static int config_get_readonly(git_config_backend *cfg, const char *key, git_con return error; } - entry->free = free_diskfile_entry; + entry->free = config_snapshot_entry_free; entry->payload = entries; *out = entry; return 0; } -static int config_set_readonly(git_config_backend *cfg, const char *name, const char *value) +static int config_snapshot_set(git_config_backend *cfg, const char *name, const char *value) { GIT_UNUSED(cfg); GIT_UNUSED(name); @@ -84,7 +84,7 @@ static int config_set_readonly(git_config_backend *cfg, const char *name, const return config_error_readonly(); } -static int config_set_multivar_readonly( +static int config_snapshot_set_multivar( git_config_backend *cfg, const char *name, const char *regexp, const char *value) { GIT_UNUSED(cfg); @@ -95,7 +95,7 @@ static int config_set_multivar_readonly( return config_error_readonly(); } -static int config_delete_multivar_readonly(git_config_backend *cfg, const char *name, const char *regexp) +static int config_snapshot_delete_multivar(git_config_backend *cfg, const char *name, const char *regexp) { GIT_UNUSED(cfg); GIT_UNUSED(name); @@ -104,7 +104,7 @@ static int config_delete_multivar_readonly(git_config_backend *cfg, const char * return config_error_readonly(); } -static int config_delete_readonly(git_config_backend *cfg, const char *name) +static int config_snapshot_delete(git_config_backend *cfg, const char *name) { GIT_UNUSED(cfg); GIT_UNUSED(name); @@ -112,14 +112,14 @@ static int config_delete_readonly(git_config_backend *cfg, const char *name) return config_error_readonly(); } -static int config_lock_readonly(git_config_backend *_cfg) +static int config_snapshot_lock(git_config_backend *_cfg) { GIT_UNUSED(_cfg); return config_error_readonly(); } -static int config_unlock_readonly(git_config_backend *_cfg, int success) +static int config_snapshot_unlock(git_config_backend *_cfg, int success) { GIT_UNUSED(_cfg); GIT_UNUSED(success); @@ -127,7 +127,7 @@ static int config_unlock_readonly(git_config_backend *_cfg, int success) return config_error_readonly(); } -static void backend_readonly_free(git_config_backend *_backend) +static void config_snapshot_free(git_config_backend *_backend) { config_snapshot_backend *backend = GIT_CONTAINER_OF(_backend, config_snapshot_backend, parent); @@ -139,7 +139,7 @@ static void backend_readonly_free(git_config_backend *_backend) git__free(backend); } -static int config_readonly_open(git_config_backend *cfg, git_config_level_t level, const git_repository *repo) +static int config_snapshot_open(git_config_backend *cfg, git_config_level_t level, const git_repository *repo) { config_snapshot_backend *b = GIT_CONTAINER_OF(cfg, config_snapshot_backend, parent); git_config_entries *entries = NULL; @@ -188,17 +188,17 @@ int git_config_backend_snapshot(git_config_backend **out, git_config_backend *so backend->parent.readonly = 1; backend->parent.version = GIT_CONFIG_BACKEND_VERSION; - backend->parent.open = config_readonly_open; - backend->parent.get = config_get_readonly; - backend->parent.set = config_set_readonly; - backend->parent.set_multivar = config_set_multivar_readonly; + backend->parent.open = config_snapshot_open; + backend->parent.get = config_snapshot_get; + backend->parent.set = config_snapshot_set; + backend->parent.set_multivar = config_snapshot_set_multivar; backend->parent.snapshot = git_config_backend_snapshot; - backend->parent.del = config_delete_readonly; - backend->parent.del_multivar = config_delete_multivar_readonly; - backend->parent.iterator = config_iterator_new_readonly; - backend->parent.lock = config_lock_readonly; - backend->parent.unlock = config_unlock_readonly; - backend->parent.free = backend_readonly_free; + backend->parent.del = config_snapshot_delete; + backend->parent.del_multivar = config_snapshot_delete_multivar; + backend->parent.iterator = config_snapshot_iterator; + backend->parent.lock = config_snapshot_lock; + backend->parent.unlock = config_snapshot_unlock; + backend->parent.free = config_snapshot_free; *out = &backend->parent; |