diff options
author | Carlos Martín Nieto <cmn@dwim.me> | 2014-12-21 15:31:03 +0000 |
---|---|---|
committer | Carlos Martín Nieto <cmn@dwim.me> | 2015-03-03 18:35:12 +0100 |
commit | 9a97f49e3aa15edc479fc590f4b28fc44c155c40 (patch) | |
tree | b85de2d396b9d0d408f688212c0cdc74347ea7b3 /src | |
parent | 76f034180aee96fcc1fffd5267ccbc6ada68482a (diff) | |
download | libgit2-9a97f49e3aa15edc479fc590f4b28fc44c155c40.tar.gz |
config: borrow refcounted referencescmn/config-borrow-entry
This changes the get_entry() method to return a refcounted version of
the config entry, which you have to free when you're done.
This allows us to avoid freeing the memory in which the entry is stored
on a refresh, which may happen at any time for a live config.
For this reason, get_string() has been forbidden on live configs and a
new function get_string_buf() has been added, which stores the string in
a git_buf which the user then owns.
The functions which parse the string value takea advantage of the
borrowing to parse safely and then release the entry.
Diffstat (limited to 'src')
-rw-r--r-- | src/attrcache.c | 3 | ||||
-rw-r--r-- | src/branch.c | 39 | ||||
-rw-r--r-- | src/checkout.c | 10 | ||||
-rw-r--r-- | src/config.c | 119 | ||||
-rw-r--r-- | src/config.h | 4 | ||||
-rw-r--r-- | src/config_cache.c | 3 | ||||
-rw-r--r-- | src/config_file.c | 24 | ||||
-rw-r--r-- | src/config_file.h | 2 | ||||
-rw-r--r-- | src/diff.c | 3 | ||||
-rw-r--r-- | src/diff_driver.c | 3 | ||||
-rw-r--r-- | src/remote.c | 9 | ||||
-rw-r--r-- | src/repository.c | 14 |
12 files changed, 165 insertions, 68 deletions
diff --git a/src/attrcache.c b/src/attrcache.c index 018fa4874..5bc260460 100644 --- a/src/attrcache.c +++ b/src/attrcache.c @@ -276,7 +276,7 @@ static int attr_cache__lookup_path( { git_buf buf = GIT_BUF_INIT; int error; - const git_config_entry *entry = NULL; + git_config_entry *entry = NULL; *out = NULL; @@ -296,6 +296,7 @@ static int attr_cache__lookup_path( else if (!git_sysdir_find_xdg_file(&buf, fallback)) *out = git_buf_detach(&buf); + git_config_entry_free(entry); git_buf_free(&buf); return error; diff --git a/src/branch.c b/src/branch.c index 4e9460f36..06602c525 100644 --- a/src/branch.c +++ b/src/branch.c @@ -301,7 +301,7 @@ int git_branch_name( } static int retrieve_upstream_configuration( - const char **out, + git_buf *out, const git_config *config, const char *canonical_branch_name, const char *format) @@ -313,7 +313,7 @@ static int retrieve_upstream_configuration( canonical_branch_name + strlen(GIT_REFS_HEADS_DIR)) < 0) return -1; - error = git_config_get_string(out, config, git_buf_cstr(&buf)); + error = git_config_get_string_buf(out, config, git_buf_cstr(&buf)); git_buf_free(&buf); return error; } @@ -323,7 +323,8 @@ int git_branch_upstream_name( git_repository *repo, const char *refname) { - const char *remote_name, *merge_name; + git_buf remote_name = GIT_BUF_INIT; + git_buf merge_name = GIT_BUF_INIT; git_buf buf = GIT_BUF_INIT; int error = -1; git_remote *remote = NULL; @@ -348,27 +349,27 @@ int git_branch_upstream_name( &merge_name, config, refname, "branch.%s.merge")) < 0) goto cleanup; - if (!*remote_name || !*merge_name) { + if (git_buf_len(&remote_name) == 0 || git_buf_len(&merge_name) == 0) { giterr_set(GITERR_REFERENCE, "branch '%s' does not have an upstream", refname); error = GIT_ENOTFOUND; goto cleanup; } - if (strcmp(".", remote_name) != 0) { - if ((error = git_remote_lookup(&remote, repo, remote_name)) < 0) + if (strcmp(".", git_buf_cstr(&remote_name)) != 0) { + if ((error = git_remote_lookup(&remote, repo, git_buf_cstr(&remote_name))) < 0) goto cleanup; - refspec = git_remote__matching_refspec(remote, merge_name); + refspec = git_remote__matching_refspec(remote, git_buf_cstr(&merge_name)); if (!refspec) { error = GIT_ENOTFOUND; goto cleanup; } - if (git_refspec_transform(&buf, refspec, merge_name) < 0) + if (git_refspec_transform(&buf, refspec, git_buf_cstr(&merge_name)) < 0) goto cleanup; } else - if (git_buf_sets(&buf, merge_name) < 0) + if (git_buf_set(&buf, git_buf_cstr(&merge_name), git_buf_len(&merge_name)) < 0) goto cleanup; error = git_buf_set(out, git_buf_cstr(&buf), git_buf_len(&buf)); @@ -376,6 +377,8 @@ int git_branch_upstream_name( cleanup: git_config_free(config); git_remote_free(remote); + git_buf_free(&remote_name); + git_buf_free(&merge_name); git_buf_free(&buf); return error; } @@ -383,29 +386,25 @@ cleanup: int git_branch_upstream_remote(git_buf *buf, git_repository *repo, const char *refname) { int error; - const char *str; git_config *cfg; if (!git_reference__is_branch(refname)) return not_a_local_branch(refname); - git_buf_sanitize(buf); - if ((error = git_repository_config_snapshot(&cfg, repo)) < 0) + if ((error = git_repository_config__weakptr(&cfg, repo)) < 0) return error; - if ((error = retrieve_upstream_configuration(&str, cfg, refname, "branch.%s.remote")) < 0) - goto cleanup; + git_buf_sanitize(buf); - if (!*str) { + if ((error = retrieve_upstream_configuration(buf, cfg, refname, "branch.%s.remote")) < 0) + return error; + + if (git_buf_len(buf) == 0) { giterr_set(GITERR_REFERENCE, "branch '%s' does not have an upstream remote", refname); error = GIT_ENOTFOUND; - goto cleanup; + git_buf_clear(buf); } - error = git_buf_puts(buf, str); - -cleanup: - git_config_free(cfg); return error; } diff --git a/src/checkout.c b/src/checkout.c index c06928138..b106c110c 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -2390,25 +2390,27 @@ static int checkout_data_init( if ((data->opts.checkout_strategy & (GIT_CHECKOUT_CONFLICT_STYLE_MERGE | GIT_CHECKOUT_CONFLICT_STYLE_DIFF3)) == 0) { - const char *conflict_style; + git_config_entry *conflict_style = NULL; git_config *cfg = NULL; if ((error = git_repository_config__weakptr(&cfg, repo)) < 0 || - (error = git_config_get_string(&conflict_style, cfg, "merge.conflictstyle")) < 0 || + (error = git_config_get_entry(&conflict_style, cfg, "merge.conflictstyle")) < 0 || error == GIT_ENOTFOUND) ; else if (error) goto cleanup; - else if (strcmp(conflict_style, "merge") == 0) + else if (strcmp(conflict_style->value, "merge") == 0) data->opts.checkout_strategy |= GIT_CHECKOUT_CONFLICT_STYLE_MERGE; - else if (strcmp(conflict_style, "diff3") == 0) + else if (strcmp(conflict_style->value, "diff3") == 0) data->opts.checkout_strategy |= GIT_CHECKOUT_CONFLICT_STYLE_DIFF3; else { giterr_set(GITERR_CHECKOUT, "unknown style '%s' given for 'merge.conflictstyle'", conflict_style); error = -1; + git_config_entry_free(conflict_style); goto cleanup; } + git_config_entry_free(conflict_style); } if ((error = git_vector_init(&data->removes, 0, git__strcmp_cb)) < 0 || diff --git a/src/config.c b/src/config.c index f80770138..d116a9d80 100644 --- a/src/config.c +++ b/src/config.c @@ -19,6 +19,14 @@ #include <ctype.h> +void git_config_entry_free(git_config_entry *entry) +{ + if (!entry) + return; + + entry->free(entry); +} + typedef struct { git_refcount rc; @@ -638,7 +646,7 @@ int git_config__update_entry( bool only_if_existing) { int error = 0; - const git_config_entry *ce = NULL; + git_config_entry *ce = NULL; if ((error = git_config__lookup_entry(&ce, config, key, false)) < 0) return error; @@ -657,6 +665,7 @@ int git_config__update_entry( else error = git_config_set_string(config, key, value); + git_config_entry_free(ce); return error; } @@ -677,7 +686,7 @@ enum { }; static int get_entry( - const git_config_entry **out, + git_config_entry **out, const git_config *cfg, const char *name, bool normalize_name, @@ -721,13 +730,13 @@ cleanup: } int git_config_get_entry( - const git_config_entry **out, const git_config *cfg, const char *name) + git_config_entry **out, const git_config *cfg, const char *name) { return get_entry(out, cfg, name, true, GET_ALL_ERRORS); } int git_config__lookup_entry( - const git_config_entry **out, + git_config_entry **out, const git_config *cfg, const char *key, bool no_errors) @@ -743,87 +752,154 @@ int git_config_get_mapped( const git_cvar_map *maps, size_t map_n) { - const git_config_entry *entry; + git_config_entry *entry; int ret; if ((ret = get_entry(&entry, cfg, name, true, GET_ALL_ERRORS)) < 0) return ret; - return git_config_lookup_map_value(out, maps, map_n, entry->value); + ret = git_config_lookup_map_value(out, maps, map_n, entry->value); + git_config_entry_free(entry); + + return ret; } int git_config_get_int64(int64_t *out, const git_config *cfg, const char *name) { - const git_config_entry *entry; + git_config_entry *entry; int ret; if ((ret = get_entry(&entry, cfg, name, true, GET_ALL_ERRORS)) < 0) return ret; - return git_config_parse_int64(out, entry->value); + ret = git_config_parse_int64(out, entry->value); + git_config_entry_free(entry); + + return ret; } int git_config_get_int32(int32_t *out, const git_config *cfg, const char *name) { - const git_config_entry *entry; + git_config_entry *entry; int ret; if ((ret = get_entry(&entry, cfg, name, true, GET_ALL_ERRORS)) < 0) return ret; - return git_config_parse_int32(out, entry->value); + ret = git_config_parse_int32(out, entry->value); + git_config_entry_free(entry); + + return ret; } int git_config_get_bool(int *out, const git_config *cfg, const char *name) { - const git_config_entry *entry; + git_config_entry *entry; int ret; if ((ret = get_entry(&entry, cfg, name, true, GET_ALL_ERRORS)) < 0) return ret; - return git_config_parse_bool(out, entry->value); + ret = git_config_parse_bool(out, entry->value); + git_config_entry_free(entry); + + return ret; +} + +static int is_readonly(const git_config *cfg) +{ + size_t i; + file_internal *internal; + + git_vector_foreach(&cfg->files, i, internal) { + if (!internal || !internal->file) + continue; + + if (!internal->file->readonly) + return 0; + } + + return 1; } int git_config_get_path(git_buf *out, const git_config *cfg, const char *name) { - const git_config_entry *entry; + git_config_entry *entry; int error; if ((error = get_entry(&entry, cfg, name, true, GET_ALL_ERRORS)) < 0) return error; - return git_config_parse_path(out, entry->value); + error = git_config_parse_path(out, entry->value); + git_config_entry_free(entry); + + return error; } int git_config_get_string( const char **out, const git_config *cfg, const char *name) { - const git_config_entry *entry; - int ret = get_entry(&entry, cfg, name, true, GET_ALL_ERRORS); + git_config_entry *entry; + int ret; + + if (!is_readonly(cfg)) { + giterr_set(GITERR_CONFIG, "get_string called on a live config object"); + return -1; + } + + ret = get_entry(&entry, cfg, name, true, GET_ALL_ERRORS); *out = !ret ? (entry->value ? entry->value : "") : NULL; + + git_config_entry_free(entry); + return ret; } -const char *git_config__get_string_force( +int git_config_get_string_buf( + git_buf *out, const git_config *cfg, const char *name) +{ + git_config_entry *entry; + int ret; + const char *str; + + git_buf_sanitize(out); + + ret = get_entry(&entry, cfg, name, true, GET_ALL_ERRORS); + str = !ret ? (entry->value ? entry->value : "") : NULL; + + if (str) + ret = git_buf_puts(out, str); + + git_config_entry_free(entry); + + return ret; +} + +char *git_config__get_string_force( const git_config *cfg, const char *key, const char *fallback_value) { - const git_config_entry *entry; + git_config_entry *entry; + char *ret; + get_entry(&entry, cfg, key, false, GET_NO_ERRORS); - return (entry && entry->value) ? entry->value : fallback_value; + ret = (entry && entry->value) ? git__strdup(entry->value) : fallback_value ? git__strdup(fallback_value) : NULL; + git_config_entry_free(entry); + + return ret; } int git_config__get_bool_force( const git_config *cfg, const char *key, int fallback_value) { int val = fallback_value; - const git_config_entry *entry; + git_config_entry *entry; get_entry(&entry, cfg, key, false, GET_NO_ERRORS); if (entry && git_config_parse_bool(&val, entry->value) < 0) giterr_clear(); + git_config_entry_free(entry); return val; } @@ -831,13 +907,14 @@ int git_config__get_int_force( const git_config *cfg, const char *key, int fallback_value) { int32_t val = (int32_t)fallback_value; - const git_config_entry *entry; + git_config_entry *entry; get_entry(&entry, cfg, key, false, GET_NO_ERRORS); if (entry && git_config_parse_int32(&val, entry->value) < 0) giterr_clear(); + git_config_entry_free(entry); return (int)val; } diff --git a/src/config.h b/src/config.h index b0dcb49ac..691868b1d 100644 --- a/src/config.h +++ b/src/config.h @@ -48,7 +48,7 @@ extern int git_config__normalize_name(const char *in, char **out); /* internal only: does not normalize key and sets out to NULL if not found */ extern int git_config__lookup_entry( - const git_config_entry **out, + git_config_entry **out, const git_config *cfg, const char *key, bool no_errors); @@ -67,7 +67,7 @@ extern int git_config__update_entry( * failures occur while trying to access the value. */ -extern const char *git_config__get_string_force( +extern char *git_config__get_string_force( const git_config *cfg, const char *key, const char *fallback_value); extern int git_config__get_bool_force( diff --git a/src/config_cache.c b/src/config_cache.c index d397a4bab..c859ec148 100644 --- a/src/config_cache.c +++ b/src/config_cache.c @@ -84,7 +84,7 @@ int git_config__cvar(int *out, git_config *config, git_cvar_cached cvar) { int error = 0; struct map_data *data = &_cvar_maps[(int)cvar]; - const git_config_entry *entry; + git_config_entry *entry; git_config__lookup_entry(&entry, config, data->cvar_name, false); @@ -96,6 +96,7 @@ int git_config__cvar(int *out, git_config *config, git_cvar_cached cvar) else error = git_config_parse_bool(out, entry->value); + git_config_entry_free(entry); return error; } diff --git a/src/config_file.c b/src/config_file.c index 168dd5483..732705687 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -96,7 +96,6 @@ typedef struct { /* mutex to coordinate accessing the values */ git_mutex values_mutex; refcounted_strmap *values; - int readonly; } diskfile_header; typedef struct { @@ -504,19 +503,26 @@ out: return ret; } +/* release the map containing the entry as an equivalent to freeing it */ +static void release_map(git_config_entry *entry) +{ + refcounted_strmap *map = (refcounted_strmap *) entry->payload; + refcounted_strmap_free(map); +} + /* * Internal function that actually gets the value in string form */ -static int config_get(git_config_backend *cfg, const char *key, const git_config_entry **out) +static int config_get(git_config_backend *cfg, const char *key, git_config_entry **out) { diskfile_header *h = (diskfile_header *)cfg; refcounted_strmap *map; git_strmap *values; khiter_t pos; cvar_t *var; - int error; + int error = 0; - if (!h->readonly && ((error = config_refresh(cfg)) < 0)) + if (!h->parent.readonly && ((error = config_refresh(cfg)) < 0)) return error; map = refcounted_strmap_take(h); @@ -534,9 +540,11 @@ static int config_get(git_config_backend *cfg, const char *key, const git_config while (var->next) var = var->next; - refcounted_strmap_free(map); *out = var->entry; - return 0; + (*out)->free = release_map; + (*out)->payload = map; + + return error; } static int config_set_multivar( @@ -763,7 +771,7 @@ static int config_readonly_open(git_config_backend *cfg, git_config_level_t leve refcounted_strmap *src_map; int error; - if (!src_header->readonly && (error = config_refresh(&src_header->parent)) < 0) + if (!src_header->parent.readonly && (error = config_refresh(&src_header->parent)) < 0) return error; /* We're just copying data, don't care about the level */ @@ -787,7 +795,7 @@ int git_config_file__snapshot(git_config_backend **out, diskfile_backend *in) backend->snapshot_from = in; - backend->header.readonly = 1; + backend->header.parent.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/src/config_file.h b/src/config_file.h index fcccbd5cc..0d8bf740f 100644 --- a/src/config_file.h +++ b/src/config_file.h @@ -21,7 +21,7 @@ GIT_INLINE(void) git_config_file_free(git_config_backend *cfg) } GIT_INLINE(int) git_config_file_get_string( - const git_config_entry **out, git_config_backend *cfg, const char *name) + git_config_entry **out, git_config_backend *cfg, const char *name) { return cfg->get(cfg, name, out); } diff --git a/src/diff.c b/src/diff.c index 815351b21..9432b0467 100644 --- a/src/diff.c +++ b/src/diff.c @@ -461,12 +461,13 @@ static int diff_list_apply_options( /* if ignore_submodules not explicitly set, check diff config */ if (diff->opts.ignore_submodules <= 0) { - const git_config_entry *entry; + git_config_entry *entry; git_config__lookup_entry(&entry, cfg, "diff.ignoresubmodules", true); if (entry && git_submodule_parse_ignore( &diff->opts.ignore_submodules, entry->value) < 0) giterr_clear(); + git_config_entry_free(entry); } /* if either prefix is not set, figure out appropriate value */ diff --git a/src/diff_driver.c b/src/diff_driver.c index 049e6ef2a..69eef0f7a 100644 --- a/src/diff_driver.c +++ b/src/diff_driver.c @@ -242,7 +242,7 @@ static int git_diff_driver_load( khiter_t pos; git_config *cfg = NULL; git_buf name = GIT_BUF_INIT; - const git_config_entry *ce; + git_config_entry *ce = NULL; bool found_driver = false; if ((reg = git_repository_driver_registry(repo)) == NULL) @@ -341,6 +341,7 @@ static int git_diff_driver_load( *out = drv; done: + git_config_entry_free(ce); git_buf_free(&name); git_config_free(cfg); diff --git a/src/remote.c b/src/remote.c index bc6d8a2c4..4924bf83a 100644 --- a/src/remote.c +++ b/src/remote.c @@ -52,7 +52,7 @@ static int add_refspec(git_remote *remote, const char *string, bool is_fetch) static int download_tags_value(git_remote *remote, git_config *cfg) { - const git_config_entry *ce; + git_config_entry *ce; git_buf buf = GIT_BUF_INIT; int error; @@ -70,6 +70,7 @@ static int download_tags_value(git_remote *remote, git_config *cfg) remote->download_tags = GIT_REMOTE_DOWNLOAD_TAGS_ALL; } + git_config_entry_free(ce); return error; } @@ -548,7 +549,7 @@ int git_remote_save(const git_remote *remote) git_config *cfg; const char *tagopt = NULL; git_buf buf = GIT_BUF_INIT; - const git_config_entry *existing; + git_config_entry *existing = NULL; assert(remote); @@ -618,6 +619,7 @@ int git_remote_save(const git_remote *remote) cfg, git_buf_cstr(&buf), tagopt, true, false); cleanup: + git_config_entry_free(existing); git_buf_free(&buf); return error; } @@ -753,7 +755,7 @@ int git_remote_ls(const git_remote_head ***out, size_t *size, git_remote *remote int git_remote__get_http_proxy(git_remote *remote, bool use_ssl, char **proxy_url) { git_config *cfg; - const git_config_entry *ce; + git_config_entry *ce = NULL; const char *val = NULL; int error; @@ -805,6 +807,7 @@ found: *proxy_url = git__strdup(val); GITERR_CHECK_ALLOC(*proxy_url); } + git_config_entry_free(ce); return 0; } diff --git a/src/repository.c b/src/repository.c index 778cdefd7..43d3aeaea 100644 --- a/src/repository.c +++ b/src/repository.c @@ -191,7 +191,7 @@ static int load_config_data(git_repository *repo, const git_config *config) static int load_workdir(git_repository *repo, git_config *config, git_buf *parent_path) { int error; - const git_config_entry *ce; + git_config_entry *ce; git_buf worktree = GIT_BUF_INIT; if (repo->is_bare) @@ -204,7 +204,7 @@ static int load_workdir(git_repository *repo, git_config *config, git_buf *paren if (ce && ce->value) { if ((error = git_path_prettify_dir( &worktree, ce->value, repo->path_repository)) < 0) - return error; + goto cleanup; repo->workdir = git_buf_detach(&worktree); } @@ -212,14 +212,18 @@ static int load_workdir(git_repository *repo, git_config *config, git_buf *paren repo->workdir = git_buf_detach(parent_path); else { if (git_path_dirname_r(&worktree, repo->path_repository) < 0 || - git_path_to_dir(&worktree) < 0) - return -1; + git_path_to_dir(&worktree) < 0) { + error = -1; + goto cleanup; + } repo->workdir = git_buf_detach(&worktree); } GITERR_CHECK_ALLOC(repo->workdir); - return 0; +cleanup: + git_config_entry_free(ce); + return error; } /* |