diff mbox series

[v2,03/20] pretty: fix memory leaks when parsing pretty formats

Message ID 8a1963685e73fecb64bebb1154ad6205caf09a8d.1724315484.git.ps@pks.im (mailing list archive)
State Accepted
Commit 60289b50d01f0b064b8139031dcd3a0d6dfa96a9
Headers show
Series Memory leak fixes (pt.5) | expand

Commit Message

Patrick Steinhardt Aug. 22, 2024, 9:17 a.m. UTC
When parsing pretty formats from the config we leak the name and user
format whenever these are set multiple times. This is because we do not
free any already-set value in case there is one.

Plugging this leak for the name is trivial. For the user format we need
to be a bit more careful, because we may end up assigning a pointer into
the allocated region when the string is prefixed with either "format" or
"tformat:". In order to make it safe to unconditionally free the user
format we thus strdup the stripped string into the field instead of a
pointer into the string.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pretty.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/pretty.c b/pretty.c
index 44222fb83c6..5e162d7204d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -63,7 +63,7 @@  static int git_pretty_formats_config(const char *var, const char *value,
 				     void *cb UNUSED)
 {
 	struct cmt_fmt_map *commit_format = NULL;
-	const char *name;
+	const char *name, *stripped;
 	char *fmt;
 	int i;
 
@@ -90,15 +90,21 @@  static int git_pretty_formats_config(const char *var, const char *value,
 		commit_formats_len++;
 	}
 
+	free((char *)commit_format->name);
 	commit_format->name = xstrdup(name);
 	commit_format->format = CMIT_FMT_USERFORMAT;
 	if (git_config_string(&fmt, var, value))
 		return -1;
 
-	if (skip_prefix(fmt, "format:", &commit_format->user_format)) {
+	free((char *)commit_format->user_format);
+	if (skip_prefix(fmt, "format:", &stripped)) {
 		commit_format->is_tformat = 0;
-	} else if (skip_prefix(fmt, "tformat:", &commit_format->user_format)) {
+		commit_format->user_format = xstrdup(stripped);
+		free(fmt);
+	} else if (skip_prefix(fmt, "tformat:", &stripped)) {
 		commit_format->is_tformat = 1;
+		commit_format->user_format = xstrdup(stripped);
+		free(fmt);
 	} else if (strchr(fmt, '%')) {
 		commit_format->is_tformat = 1;
 		commit_format->user_format = fmt;