Re: [PATCH 3/3] fetch: treat --tags like refs/tags/*:refs/tags/* when pruning

["Carlos Martín Nieto" <cmn@elego.de>]

[-- Attachment #1: Type: text/plain, Size: 1392 bytes --]

On Fri, 2011-10-07 at 12:33 -0400, Jeff King wrote:
> On Thu, Oct 06, 2011 at 11:21:47PM +0200, Carlos Martín Nieto wrote:
> 
> > diff --git a/builtin/fetch.c b/builtin/fetch.c
> > index b937d71..94b2bd3 100644
> > --- a/builtin/fetch.c
> > +++ b/builtin/fetch.c
> > @@ -699,8 +699,17 @@ static int do_fetch(struct transport *transport,
> >  		free_refs(ref_map);
> >  		return 1;
> >  	}
> > -	if (prune)
> > +	if (prune) {
> > +		/* If --tags was specified, we need to tell prune_refs
> > +		 * that we're filtering the refs from the remote */
> > +		if (tags == TAGS_SET) {
> > +			const char * tags_refspec = "refs/tags/*:refs/tags/*";
> > +			refs = xrealloc(refs, (ref_count + 1) * sizeof(struct refspec));
> > +			refs[ref_count] = *parse_fetch_refspec(1, &tags_refspec);
> > +			ref_count++;
> > +		}
> >  		prune_refs(transport, refs, ref_count, ref_map);
> > +	}
> 
> I don't think we can realloc refs here. It's passed into do_fetch. When
> we realloc it, the old pointer value will be invalid. But when we return
> from do_fetch, the caller (fetch_one) will still have that old value,
> and will call free() on it.

Yes, you're right. I guess it's been working by luck and generous amount
of memory.

> 
> Instead, you have to make a whole new list, copy the old values in, add
> your new one, and then free the result.

Will do.

   cmn


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]