git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 0/7] leak fixes

From
Jeff King <peff@peff.net>
Date
Aug 17, 2020, 21:32 UTC
Message-ID
<20200817213228.GA1854603@coredump.intra.peff.net>
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
Previous: Jeff KingNext: Jeff King
Message 13 of 20 in “more small leak fixes”
  1. 0/6 more small leak fixesJeff King, Aug 14, 2020
  2. 1/6 submodule--helper: use strbuf_release() to free strbufsJeff King, Aug 14, 2020
  3. 2/6 checkout: fix leak of non-existent branch namesJeff King, Aug 14, 2020
  4. 3/6 config: fix leaks from git_config_get_string_const()Jeff King, Aug 14, 2020
  5. 4/6 config: drop git_config_get_string_const()Jeff King, Aug 14, 2020
  6. Junio C HamanoAug 14, 2020
  7. Jeff KingAug 15, 2020
  8. Junio C HamanoAug 17, 2020
  9. 5/6 config: fix leak in git_config_get_expiry_in_days()Jeff King, Aug 14, 2020
  10. 6/6 submodule--helper: fix leak of core.worktree valueJeff King, Aug 14, 2020
  11. Jeff KingAug 14, 2020
  12. Jeff KingAug 14, 2020
  13. 0/7 leak fixesJeff King, Aug 17, 2020
  14. 1/7 clear_pattern_list(): clear embedded hashmapsJeff King, Aug 17, 2020
  15. 2/7 submodule--helper: use strbuf_release() to free strbufsJeff King, Aug 17, 2020
  16. 3/7 checkout: fix leak of non-existent branch namesJeff King, Aug 17, 2020
  17. 4/7 config: fix leaks from git_config_get_string_const()Jeff King, Aug 17, 2020
  18. 6/7 config: fix leak in git_config_get_expiry_in_days()Jeff King, Aug 17, 2020
  19. 7/7 submodule--helper: fix leak of core.worktree valueJeff King, Aug 17, 2020
  20. 5/7 config: drop git_config_get_string_const()Jeff King, Aug 17, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.