From ef4fe5617eadc24fd09ce7778b77bb87f147480c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 25 May 2017 15:33:05 -0400 Subject: [PATCH] connect.c: fix leak in parse_one_symref_info() If we successfully parse a symref value like "HEAD:refs/heads/master", we add the result to a string list. But because the string list is marked STRING_LIST_INIT_DUP, the string list code will make a copy of the string and add the copy. This patch fixes it by adding the entry with string_list_append_nodup(), which lets the string list take ownership of our newly allocated string. There are two alternatives that seem like they would work, but aren't the right solution. The first is to initialize the list with the "NODUP" initializer. That would avoid the copy, but then the string list would not realize that it owns the strings. When we eventually call string_list_clear(), it would not free the strings, causing a leak. The second option would be to use the normal string_list_append(), but free the local copy in our function. We can't do this because the local copy actually contains _two_ strings; the symref name and its target. We point to the target pointer via the "util" field, and its memory must last as long as the string list does. You may also wonder whether it's safe to ever free the local copy, since the target points into it. The answer is yes, because we duplicate it in annotaate_refs_with_symref_info before clearing the string list. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- connect.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/connect.c b/connect.c index 8cb93b0720..8af002bf0f 100644 --- a/connect.c +++ b/connect.c @@ -71,7 +71,7 @@ static void parse_one_symref_info(struct string_list *symref, const char *val, i check_refname_format(target, REFNAME_ALLOW_ONELEVEL)) /* "symref=bogus:pair */ goto reject; - item = string_list_append(symref, sym); + item = string_list_append_nodup(symref, sym); item->util = target; return; reject: