@matheusbsbs / matheus-bagliano / commits / 648abbe22d

config: fix leaking comment character config

When the comment line character has been specified multiple times in the configuration, then `git_default_core_config()` will cause a memory leak because it unconditionally copies the string into `comment_line_str` without free'ing the previous value. In fact, it can't easily free the value in the first place because it may contain a string constant. Refactor the code such that we track allocated comment character strings via a separate non-constant variable `comment_line_str_to_free`. Adapt sites that set `comment_line_str` to set both and free the old value that was stored in `comment_line_str_to_free`. This memory leak is being hit in t3404. As there are still other memory leaks in that file we cannot yet mark it as passing with leak checking enabled. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Aug 14, 2024 at 08:52 UTC 648abbe22d55a6004bf6dafa7c0ed209572c9fe9

4 files changed +9 -3

builtin/commit.c

+5 -2

@@ -684,7 +684,9 @@ static void adjust_comment_line_char(const struct strbuf *sb)

684 const char *p;

685

686 if (!memchr(sb->buf, candidates[0], sb->len)) {

687 - comment_line_str = xstrfmt("%c", candidates[0]);

687 + free(comment_line_str_to_free);

688 + comment_line_str = comment_line_str_to_free =

689 + xstrfmt("%c", candidates[0]);

690 return;

691 }

692

@@ -705,7 +707,8 @@ static void adjust_comment_line_char(const struct strbuf *sb)

707 if (!*p)

708 die(_("unable to select a comment character that is not used\n"

709 "in the current commit message"));

708 - comment_line_str = xstrfmt("%c", *p);

710 + free(comment_line_str_to_free);

711 + comment_line_str = comment_line_str_to_free = xstrfmt("%c", *p);

712 }

713

714 static void prepare_amend_commit(struct commit *commit, struct strbuf *sb,

config.c

+2 -1

@@ -1596,7 +1596,8 @@ static int git_default_core_config(const char *var, const char *value,

1596 else if (value[0]) {

1597 if (strchr(value, '\n'))

1598 return error(_("%s cannot contain newline"), var);

1599 - comment_line_str = xstrdup(value);

1599 + comment_line_str = value;

1600 + FREE_AND_NULL(comment_line_str_to_free);

1601 auto_comment_line_char = 0;

1602 } else

1603 return error(_("%s must have at least one character"), var);

environment.c

@@ -114,6 +114,7 @@ int protect_ntfs = PROTECT_NTFS_DEFAULT;

114 * that is subject to stripspace.

115 */

116 const char *comment_line_str = "#";

117 +char *comment_line_str_to_free;

118 int auto_comment_line_char;

119

120 /* Parallel index stat data preload? */

environment.h

@@ -9,6 +9,7 @@ struct strvec;

9 * that is subject to stripspace.

10 */

11 extern const char *comment_line_str;

12 +extern char *comment_line_str_to_free;

13 extern int auto_comment_line_char;

15 /*