git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] connect.c: fix leak in parse_one_symref_info()
@ 2017-05-25 19:33 Jeff King
  0 siblings, 0 replies; only message in thread
From: Jeff King @ 2017-05-25 19:33 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

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 <peff@peff.net>
---
Phew. For a one-line leak fix, that sure was complicated.

I doubt it matters much in practice, because servers send only a single
HEAD, so we just leak one string. But I happened to notice it while
looking at the unborn-HEAD thing.

 connect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/connect.c b/connect.c
index cd21a1b6f..c72b1d115 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:
-- 
2.13.0.496.ge44ba89db

^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2017-05-25 19:33 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-25 19:33 [PATCH] connect.c: fix leak in parse_one_symref_info() Jeff King

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).