[v2,0/7] leak fixes

Message

Jeff King Aug. 17, 2020, 9:32 p.m. UTC

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