git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: [PATCH 4/6] config: drop git_config_get_string_const()
Date: Sat, 15 Aug 2020 02:34:37 -0400	[thread overview]
Message-ID: <20200815063437.GA619381@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqq364pvv03.fsf@gitster.c.googlers.com>

On Fri, Aug 14, 2020 at 01:21:32PM -0700, Junio C Hamano wrote:

> 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,

  reply	other threads:[~2020-08-15 23:23 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-14 16:13 [PATCH 0/6] more small leak fixes Jeff King
2020-08-14 16:14 ` [PATCH 1/6] submodule--helper: use strbuf_release() to free strbufs Jeff King
2020-08-14 16:14 ` [PATCH 2/6] checkout: fix leak of non-existent branch names Jeff King
2020-08-14 16:17 ` [PATCH 3/6] config: fix leaks from git_config_get_string_const() Jeff King
2020-08-14 16:19 ` [PATCH 4/6] config: drop git_config_get_string_const() Jeff King
2020-08-14 20:21   ` Junio C Hamano
2020-08-15  6:34     ` Jeff King [this message]
2020-08-17 17:36       ` Junio C Hamano
2020-08-14 16:19 ` [PATCH 5/6] config: fix leak in git_config_get_expiry_in_days() Jeff King
2020-08-14 16:20 ` [PATCH 6/6] submodule--helper: fix leak of core.worktree value Jeff King
2020-08-14 16:25 ` [PATCH 0/6] more small leak fixes Jeff King
2020-08-14 16:27   ` Jeff King
2020-08-17 21:32 ` [PATCH v2 0/7] " Jeff King
2020-08-17 21:33   ` [PATCH v2 1/7] clear_pattern_list(): clear embedded hashmaps Jeff King
2020-08-17 21:33   ` [PATCH v2 2/7] submodule--helper: use strbuf_release() to free strbufs Jeff King
2020-08-17 21:33   ` [PATCH v2 3/7] checkout: fix leak of non-existent branch names Jeff King
2020-08-17 21:33   ` [PATCH v2 4/7] config: fix leaks from git_config_get_string_const() Jeff King
2020-08-17 21:33   ` [PATCH v2 5/7] config: drop git_config_get_string_const() Jeff King
2020-08-17 21:33   ` [PATCH v2 6/7] config: fix leak in git_config_get_expiry_in_days() Jeff King
2020-08-17 21:33   ` [PATCH v2 7/7] submodule--helper: fix leak of core.worktree value Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200815063437.GA619381@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sunshine@sunshineco.com \
    --subject='Re: [PATCH 4/6] config: drop git_config_get_string_const()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).