git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: [PATCH v2 0/7] leak fixes
Date: Mon, 17 Aug 2020 17:32:28 -0400	[thread overview]
Message-ID: <20200817213228.GA1854603@coredump.intra.peff.net> (raw)
In-Reply-To: <20200814161328.GA153929@coredump.intra.peff.net>

On Fri, Aug 14, 2020 at 12:13:28PM -0400, Jeff King wrote:

> There were quite a few false positives, but it did actually uncover some
> legitimate leaks. This series fixes those. I did it independently of the
> leak-fix in [2], but it would be fine to just lump it all together as
> one topic.

Here's a re-roll that drops the repo_config_get_string_const() and
git_configset_get_string_const() helpers in patch 4, as noticed by
Junio. That's the only patch with changes (range-diff below).

(I also rolled in the earlier leak-fix as "patch 1", which matches what
got queued in jk/leakfix).

  [1/7]: clear_pattern_list(): clear embedded hashmaps
  [2/7]: submodule--helper: use strbuf_release() to free strbufs
  [3/7]: checkout: fix leak of non-existent branch names
  [4/7]: config: fix leaks from git_config_get_string_const()
  [5/7]: config: drop git_config_get_string_const()
  [6/7]: config: fix leak in git_config_get_expiry_in_days()
  [7/7]: submodule--helper: fix leak of core.worktree value

 Documentation/MyFirstContribution.txt |  4 +--
 apply.c                               |  4 +--
 builtin/checkout.c                    |  4 ++-
 builtin/fetch.c                       |  2 +-
 builtin/submodule--helper.c           | 16 ++++-----
 cache.h                               |  4 +--
 checkout.c                            |  3 +-
 config.c                              | 47 +++++++++++++++++----------
 config.h                              | 15 +++++----
 connect.c                             |  4 +--
 dir.c                                 |  2 ++
 editor.c                              |  2 +-
 environment.c                         |  4 +--
 help.c                                |  2 +-
 protocol.c                            |  2 +-
 submodule.c                           |  4 +--
 t/helper/test-config.c                |  2 +-
 17 files changed, 69 insertions(+), 52 deletions(-)

1:  7a878fa4d3 = 1:  24248d0203 clear_pattern_list(): clear embedded hashmaps
2:  84d8edd867 = 2:  5bdbb11601 submodule--helper: use strbuf_release() to free strbufs
3:  76ae8ffb83 = 3:  44f3539461 checkout: fix leak of non-existent branch names
4:  e8f85a47ff = 4:  b1b5444ca1 config: fix leaks from git_config_get_string_const()
5:  9875ddeab3 ! 5:  a82f9724b6 config: drop git_config_get_string_const()
    @@ Commit message
         can swap that call out for the non-allocating "tmp" variant, which fits
         well in the example given.
     
    +    We'll drop the "configset" and "repo" variants, as well (which are
    +    unused).
    +
         Note that this frees up the "const" name, so we could rename the "tmp"
         variant back to that. But let's give some time for topics in flight to
         adapt to the new code before doing so (if we do it too soon, the
    @@ checkout.c: const char *unique_tracking_name(const char *name, struct object_id
      		free(cb_data.default_dst_oid);
     
      ## config.c ##
    +@@ config.c: 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)
    + {
    +@@ config.c: 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)
    + {
     @@ config.c: const struct string_list *git_config_get_value_multi(const char *key)
      	return repo_config_get_value_multi(the_repository, key);
      }
    @@ config.c: int git_config_get_pathname(const char *key, const char **dest)
      	if (strcmp(*output, "now")) {
     
      ## config.h ##
    +@@ config.h: 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);
    +@@ config.h: 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,
     @@ config.h: void git_config_clear(void);
       * error message and returns -1. When the configuration variable `key` is
       * not found, returns 1 without touching `dest`.
6:  ee69df55de = 6:  7eefd80257 config: fix leak in git_config_get_expiry_in_days()
7:  c4fd15a2ac = 7:  feb66301fc submodule--helper: fix leak of core.worktree value

  parent reply	other threads:[~2020-08-17 21:32 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
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 ` Jeff King [this message]
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=20200817213228.GA1854603@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sunshine@sunshineco.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public 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).