git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 6/6] determine_author_info: copy getenv output

From
Jeff King <peff@peff.net>
Date
Aug 27, 2014, 07:57 UTC
Message-ID
<20140827075756.GF26384@peff.net>
In-Reply-To
<20140827075503.GA19521@peff.net>

When figuring out the author name for a commit, we may end up either pointing to const storage from getenv("GIT_AUTHOR_*"), or to newly allocated storage based on an existing commit or the --author option.

Using const pointers to getenv's return has two problems:
  1. It is not guaranteed that the return value from getenv
     remains valid across multiple calls.
  2. We do not know whether to free the values at the end,
     so we just leak them.

We can solve both by duplicating the string returned by getenv().

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/commit.c | 40 +++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 57b046b..7f9f071 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -535,15 +535,26 @@ static int parse_force_date(const char *in, struct strbuf *out)
 	return 0;
 }
 
+static void set_ident_var(char **buf, char *val)
+{
+	free(*buf);
+	*buf = val;
+}
+
+static char *envdup(const char *var)
+{
+	const char *val = getenv(var);
+	return val ? xstrdup(val) : NULL;
+}
+
 static void determine_author_info(struct strbuf *author_ident)
 {
 	char *name, *email, *date;
 	struct ident_split author;
-	struct strbuf date_buf = STRBUF_INIT;
 
-	name = getenv("GIT_AUTHOR_NAME");
-	email = getenv("GIT_AUTHOR_EMAIL");
-	date = getenv("GIT_AUTHOR_DATE");
+	name = envdup("GIT_AUTHOR_NAME");
+	email = envdup("GIT_AUTHOR_EMAIL");
+	date = envdup("GIT_AUTHOR_DATE");
 
 	if (author_message) {
 		struct ident_split ident;
@@ -556,15 +567,16 @@ static void determine_author_info(struct strbuf *author_ident)
 		if (split_ident_line(&ident, a, len) < 0)
 			die(_("commit '%s' has malformed author line"), author_message);
 
-		name = xmemdupz(ident.name_begin, ident.name_end - ident.name_begin);
-		email = xmemdupz(ident.mail_begin, ident.mail_end - ident.mail_begin);
+		set_ident_var(&name, xmemdupz(ident.name_begin, ident.name_end - ident.name_begin));
+		set_ident_var(&email, xmemdupz(ident.mail_begin, ident.mail_end - ident.mail_begin));
+
 		if (ident.date_begin) {
-			strbuf_reset(&date_buf);
+			struct strbuf date_buf = STRBUF_INIT;
 			strbuf_addch(&date_buf, '@');
 			strbuf_add(&date_buf, ident.date_begin, ident.date_end - ident.date_begin);
 			strbuf_addch(&date_buf, ' ');
 			strbuf_add(&date_buf, ident.tz_begin, ident.tz_end - ident.tz_begin);
-			date = date_buf.buf;
+			set_ident_var(&date, strbuf_detach(&date_buf, NULL));
 		}
 	}
 
@@ -573,15 +585,15 @@ static void determine_author_info(struct strbuf *author_ident)
 
 		if (split_ident_line(&ident, force_author, strlen(force_author)) < 0)
 			die(_("malformed --author parameter"));
-		name = xmemdupz(ident.name_begin, ident.name_end - ident.name_begin);
-		email = xmemdupz(ident.mail_begin, ident.mail_end - ident.mail_begin);
+		set_ident_var(&name, xmemdupz(ident.name_begin, ident.name_end - ident.name_begin));
+		set_ident_var(&email, xmemdupz(ident.mail_begin, ident.mail_end - ident.mail_begin));
 	}
 
 	if (force_date) {
-		strbuf_reset(&date_buf);
+		struct strbuf date_buf = STRBUF_INIT;
 		if (parse_force_date(force_date, &date_buf))
 			die(_("invalid date format: %s"), force_date);
-		date = date_buf.buf;
+		set_ident_var(&date, strbuf_detach(&date_buf, NULL));
 	}
 
 	strbuf_addstr(author_ident, fmt_ident(name, email, date, IDENT_STRICT));
@@ -592,7 +604,9 @@ static void determine_author_info(struct strbuf *author_ident)
 		export_one("GIT_AUTHOR_DATE", author.date_begin, author.tz_end, '@');
 	}
 
-	strbuf_release(&date_buf);
+	free(name);
+	free(email);
+	free(date);
 }
 
 static void split_ident_or_die(struct ident_split *id, const struct strbuf *buf)
-- 
2.1.0.346.ga0367b9
Previous: Jeff KingNext: Christian Couder
Message 15 of 18 in “clean up author parsing”
  1. 0/6 clean up author parsingJeff King, Aug 27, 2014
  2. 1/6 commit: provide a function to find a header in a bufferJeff King, Aug 27, 2014
  3. Junio C HamanoAug 27, 2014
  4. Jeff KingAug 27, 2014
  5. Jeff KingAug 27, 2014
  6. Junio C HamanoAug 27, 2014
  7. Jeff KingAug 27, 2014
  8. Junio C HamanoAug 27, 2014
  9. Jeff KingAug 27, 2014
  10. Junio C HamanoAug 27, 2014
  11. 2/6 record_author_info: fix memory leak on malformed commitJeff King, Aug 27, 2014
  12. 3/6 record_author_info: use find_commit_headerJeff King, Aug 27, 2014
  13. 4/6 use strbufs in date functionsJeff King, Aug 27, 2014
  14. 5/6 determine_author_info: reuse parsing functionsJeff King, Aug 27, 2014
  15. 6/6 determine_author_info: copy getenv outputJeff King, Aug 27, 2014
  16. Christian CouderAug 27, 2014
  17. Jeff KingAug 27, 2014
  18. Junio C HamanoAug 27, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.