Re: [PATCH 4/6] config: drop git_config_get_string_const()
- From
Jeff King <peff@peff.net>
- Date
- Aug 15, 2020, 06:34 UTC
- Message-ID
- <20200815063437.GA619381@coredump.intra.peff.net>
- In-Reply-To
- <xmqq364pvv03.fsf@gitster.c.googlers.com>
On Fri, Aug 14, 2020 at 01:21:32PM -0700, Junio C Hamano wrote:
Show 12 quoted lines
> Jeff King <peff@peff.net> writes: > > > As evidenced by the leak fixes in the previous commit, the "const" in > > git_config_get_string_const() clearly misleads people into thinking that > > it does not allocate a copy of the string. We can fix this by renaming > > it, but it's easier still to just drop it. > > This turns out to be a bit more painful than I imagined. > > Do we want to do the same with repo_config_get_string_const(), by > the way? Which would open the wound even wider, but may be worth > doing for consistency.
Whoops, I actually meant to do so (and made sure we didn't have any callers) but forgot to actually include it in patch 4. It doesn't make any sense to keep those other variants.
The diff is pretty straightforward (see below). I'll include it in a re-roll (squashed into patch 4), but will wait for any other comments.
It doesn't look like there's any extra fallout from merging this with "seen".
diff --git a/config.c b/config.c index 8bb1945aa9..f0367c76ad 100644 --- a/config.c +++ b/config.c @@ -2006,20 +2006,15 @@ const struct string_list *git_configset_get_value_multi(struct config_set *cs, c return e ? &e->value_list : NULL; } -int git_configset_get_string_const(struct config_set *cs, const char *key, const char **dest) +int git_configset_get_string(struct config_set *cs, const char *key, char **dest) { const char *value; if (!git_configset_get_value(cs, key, &value)) - return git_config_string(dest, key, value); + return git_config_string((const char **)dest, key, value); else return 1; } -int git_configset_get_string(struct config_set *cs, const char *key, char **dest) -{ - return git_configset_get_string_const(cs, key, (const char **)dest); -} - int git_configset_get_string_tmp(struct config_set *cs, const char *key, const char **dest) { @@ -2161,24 +2156,17 @@ const struct string_list *repo_config_get_value_multi(struct repository *repo, return git_configset_get_value_multi(repo->config, key); } -int repo_config_get_string_const(struct repository *repo, - const char *key, const char **dest) +int repo_config_get_string(struct repository *repo, + const char *key, char **dest) { int ret; git_config_check_init(repo); - ret = git_configset_get_string_const(repo->config, key, dest); + ret = git_configset_get_string(repo->config, key, dest); if (ret < 0) git_die_config(key, NULL); return ret; } -int repo_config_get_string(struct repository *repo, - const char *key, char **dest) -{ - git_config_check_init(repo); - return repo_config_get_string_const(repo, key, (const char **)dest); -} - int repo_config_get_string_tmp(struct repository *repo, const char *key, const char **dest) { diff --git a/config.h b/config.h index d4d22c34c6..91cdfbfb41 100644 --- a/config.h +++ b/config.h @@ -458,7 +458,6 @@ void git_configset_clear(struct config_set *cs); */ int git_configset_get_value(struct config_set *cs, const char *key, const char **dest); -int git_configset_get_string_const(struct config_set *cs, const char *key, const char **dest); int git_configset_get_string(struct config_set *cs, const char *key, char **dest); int git_configset_get_string_tmp(struct config_set *cs, const char *key, const char **dest); int git_configset_get_int(struct config_set *cs, const char *key, int *dest); @@ -475,8 +474,6 @@ int repo_config_get_value(struct repository *repo, const char *key, const char **value); const struct string_list *repo_config_get_value_multi(struct repository *repo, const char *key); -int repo_config_get_string_const(struct repository *repo, - const char *key, const char **dest); int repo_config_get_string(struct repository *repo, const char *key, char **dest); int repo_config_get_string_tmp(struct repository *repo,