[02/11] config: fix a leak in git_config_copy_or_rename_section_in_file

Commit Message

Rubén Justo June 11, 2023, 6:49 p.m. UTC

In 52d59cc645 (branch: add a --copy (-c) option to go with --move (-m),
2017-06-18) a new strbuf variable was introduced, but not released.

Thus, when copying a branch that has any configuration, we have a
leak.

   $ git branch foo
   $ git config branch.foo.some-key some_value
   $ git branch -c foo bar

   Direct leak of 65 byte(s) in 1 object(s) allocated from:
       ... in xrealloc wrapper.c
       ... in strbuf_grow strbuf.c
       ... in strbuf_vaddf strbuf.c
       ... in strbuf_addf strbuf.c
       ... in store_create_section config.c
       ... in git_config_copy_or_rename_section_in_file config.c
       ... in git_config_copy_section_in_file config.c
       ... in git_config_copy_section config.c
       ... in copy_or_rename_branch builtin/branch.c
       ... in cmd_branch builtin/branch.c
       ... in run_builtin git.c

Let's fix that leak.

Signed-off-by: Rubén Justo <[e-mail]>
---
 config.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Jeff King June 12, 2023, 3:14 a.m. UTC | #1

On Sun, Jun 11, 2023 at 08:49:28PM +0200, Rubén Justo wrote:

> In 52d59cc645 (branch: add a --copy (-c) option to go with --move (-m),
> 2017-06-18) a new strbuf variable was introduced, but not released.
> 
> Thus, when copying a branch that has any configuration, we have a
> leak.
> 
>    $ git branch foo
>    $ git config branch.foo.some-key some_value
>    $ git branch -c foo bar

Looks good. Thanks for digging up the commit which introduced the
problem. I always find that gives me more confidence that the leak is
just a simple "oops, we forgot to free", and there isn't something more
subtle going on.

-Peff

Patch

diff --git a/config.c b/config.c
index b79baf83e3..39a7d7422c 100644
--- a/config.c
+++ b/config.c
@@ -3879,6 +3879,7 @@  static int git_config_copy_or_rename_section_in_file(const char *config_filename
 	free(filename_buf);
 	config_store_data_clear(&store);
 	strbuf_release(&buf);
+	strbuf_release(&copystr);
 	return ret;
 }