Re: [PATCH 1/3] ref-filter: free memory from used_atom

From: Jeff King <peff@peff.net>
Date: Oct 18, 2018, 06:33 UTC
Message-ID: <20181018063306.GA23497@sigill.intra.peff.net>
In-Reply-To: <xmqqa7njevu2.fsf@gitster-ct.c.googlers.com>

On Fri, Oct 12, 2018 at 11:35:01AM +0900, Junio C Hamano wrote:

Show 7 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:
> 
> These three patches seem to cause t6300 to fail with an attempt to
> free an invalid pointer in "git for-each-ref --format='%(push)'"
> (6300.25)

I dug into this a bit. I think it's actually a misapplication of the patches on your side. Applying them locally works, but your ot/ref-filter-plug-leaks branch does not.

The patch below on top of your branch helps. :)

diff --git a/ref-filter.c b/ref-filter.c
index f4ff80eca0..4255de1d75 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1567,7 +1567,6 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 					continue;
 			}
 			free((char *)v->s); /* we will definitely re-init it on the next line */
-			free((char *)v->s);
 			fill_remote_ref_details(atom, refname, branch, &v->s);
 			continue;
 		} else if (starts_with(name, "color:")) {

Presumably it came from the manual comment-style fixup.

With that fix, the tests run fine for me under ASan/UBSan (with the
exception of t5310, but that's fixed already in a parallel topic).

-Peff