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>

diff --git a/builtin/commit.c b/builtin/commit.c
index 66427ba82d5b9b6cf71a8a94338bfd5720d798cd..b2033c488778ff79c990c23a95f28d10898e2874 100644 (file)
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -684,7 +684,9 @@ static void adjust_comment_line_char(const struct strbuf *sb)
        const char *p;
 
        if (!memchr(sb->buf, candidates[0], sb->len)) {
-               comment_line_str = xstrfmt("%c", candidates[0]);
+               free(comment_line_str_to_free);
+               comment_line_str = comment_line_str_to_free =
+                       xstrfmt("%c", candidates[0]);
                return;
        }
 
@@ -705,7 +707,8 @@ static void adjust_comment_line_char(const struct strbuf *sb)
        if (!*p)
                die(_("unable to select a comment character that is not used\n"
                      "in the current commit message"));
-       comment_line_str = xstrfmt("%c", *p);
+       free(comment_line_str_to_free);
+       comment_line_str = comment_line_str_to_free = xstrfmt("%c", *p);
 }
 
 static void prepare_amend_commit(struct commit *commit, struct strbuf *sb,

diff --git a/config.c b/config.c
index 6421894614092428780e80b57d2d3f0ca21c975c..205660a8fba5ecb356e95306ee0349fad763b432 100644 (file)
--- a/config.c
+++ b/config.c
@@ -1596,7 +1596,8 @@ static int git_default_core_config(const char *var, const char *value,
                else if (value[0]) {
                        if (strchr(value, '\n'))
                                return error(_("%s cannot contain newline"), var);
-                       comment_line_str = xstrdup(value);
+                       comment_line_str = value;
+                       FREE_AND_NULL(comment_line_str_to_free);
                        auto_comment_line_char = 0;
                } else
                        return error(_("%s must have at least one character"), var);

diff --git a/environment.c b/environment.c
index 5cea2c9f5473e3a0739c8cf3b259c1bead877331..1d6c48b52dfc05fe10783c9b252ffe138f5173d8 100644 (file)
--- a/environment.c
+++ b/environment.c
@@ -114,6 +114,7 @@ int protect_ntfs = PROTECT_NTFS_DEFAULT;
  * that is subject to stripspace.
  */
 const char *comment_line_str = "#";
+char *comment_line_str_to_free;
 int auto_comment_line_char;
 
 /* Parallel index stat data preload? */

diff --git a/environment.h b/environment.h
index e9f01d4d11c01060e591ddc503e0bd9b73aeaddd..0148738ed63f987d81747afe9483097ecbf3fc8e 100644 (file)
--- a/environment.h
+++ b/environment.h
@@ -9,6 +9,7 @@ struct strvec;
  * that is subject to stripspace.
  */
 extern const char *comment_line_str;
+extern char *comment_line_str_to_free;
 extern int auto_comment_line_char;
 
 /*