Re: [PATCH] Fixed crash in fetching remote tags when there is not tags.

From: Alex Riesen <raa.lkml@gmail.com>
Date: Oct 10, 2007, 21:27 UTC
Message-ID: <20071010212735.GB16635@steel.home>
In-Reply-To: <20071010051034.GA30834@coredump.intra.peff.net>

Jeff King, Wed, Oct 10, 2007 07:10:35 +0200:

Show 9 quoted lines

> On Tue, Oct 09, 2007 at 08:20:43PM +0200, Alex Riesen wrote:
> 
> > Your test does not crash on my system and your fix is obviously bogus.
> > Just take a look at the only call site of the tail_link_ref: ret
> > cannot be NULL. alloc_ref will crash in memset, if this were the case.
> 
> His work is almost certainly on top of next, which crashes reliably with
> the test and has an additional call site for tail_link_ref. Aside from
> some trailing whitespace in the patch, I think his fix is reasonable.

Ach, I see.

Still, I'd suggest move the test into the caller, firstly because it is the only place that special. Also, I can't think of a proper reason to add a NULL ref to a reflist, and so the crashing tail_link_ref will help us find the callers which use tail_link_ref incorrectly (illogically too).

As the result of patter expansion can be NULL (empty pattern, as it seems), lets just check for it. I parked the patch below locally.

diff --git a/remote.c b/remote.c
index 5e92378..58d63ed 100644
--- a/remote.c
+++ b/remote.c
@@ -884,7 +884,8 @@ int get_fetch_map(struct ref *remote_refs,
 			    rm->peer_ref->name);
 	}
 
-	tail_link_ref(ref_map, tail);
+	if (ref_map)
+		tail_link_ref(ref_map, tail);
 
 	return 0;
 }