Thread (1 message) 1 message, 1 author, 2016-06-15

Re: [PATCH] fmt-merge-msg: plug small leak of commit buffer

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:04:29

Jeff King [off-list ref] writes:
quoted hunk
I note that record_person does not seem to care about the commit at all,
so an alternative fix would be:
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 1d962dc..9f0e608 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -223,16 +223,14 @@ static void add_branch_desc(struct strbuf *out, const char *name)
 
 #define util_as_integral(elem) ((intptr_t)((elem)->util))
 
-static void record_person(int which, struct string_list *people,
-			  struct commit *commit)
+static void record_person_from_buf(int which, struct string_list *people,
+				   const char *buffer)
 {
-	const char *buffer;
 	char *name_buf, *name, *name_end;
 	struct string_list_item *elem;
 	const char *field;
 
 	field = (which == 'a') ? "\nauthor " : "\ncommitter ";
-	buffer = get_commit_buffer(commit, NULL);
 	name = strstr(buffer, field);
 	if (!name)
 		return;
@@ -245,7 +243,6 @@ static void record_person(int which, struct string_list *people,
 	if (name_end < name)
 		return;
 	name_buf = xmemdupz(name, name_end - name + 1);
-	unuse_commit_buffer(commit, buffer);
 
 	elem = string_list_lookup(people, name_buf);
 	if (!elem) {
@@ -256,6 +253,14 @@ static void record_person(int which, struct string_list *people,
 	free(name_buf);
 }
 
+static void record_person(int which, struct string_list *people,
+			  struct commit *commit)
+{
+	const char *buf = get_commit_buffer(commit, NULL);
+	record_person_from_buf(which, people, buf);
+	unuse_commit_buffer(commit, buf);
+}
+
 static int cmp_string_list_util_as_integral(const void *a_, const void *b_)
 {
 	const struct string_list_item *a = a_, *b = b_;

This has the slight advantage that it adapts naturally if record_person
grows more exits, but I don't think it is a big deal either way (it only
matters if the new exit fails to copy the surrounding code and use "goto
leave").
Yeah, let me steal that from you.

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help