Re: [PATCHv3 4/4] clone: use free_refspec() to free refspec list

> On Mon, May 15, 2017 at 1:05 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > From: Jeff King <peff@peff.net>
> >
> > Using free() on a refspec was always leaky, as its string
> > fields also need freed. But it became more so when ad00f128d
> > (clone: respect configured fetch respecs during initial
> > fetch, 2016-03-30) taught clone to create a list of
> > refspecs, each of which need to be freed.
> >
> > [sg: adjusted the function parameters.]
> >
> > Signed-off-by: Jeff King <peff@peff.net>
> > Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> > ---
> >  builtin/clone.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/builtin/clone.c b/builtin/clone.c
> > index 4144190da..4bf28d7f5 100644
> > --- a/builtin/clone.c
> > +++ b/builtin/clone.c
> > @@ -1120,6 +1120,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> >         strbuf_release(&value);
> >         junk_mode = JUNK_LEAVE_ALL;
> >
> > -       free(refspec);
> > +       free_refspec(remote->fetch_refspec_nr + 1, remote->fetch);
> >         return err;
> >  }
> 
> Erm...  I should have given a bit more thought to this last patch,
> shouldn't I?
> 
> First, the unchanged commit message is now (i.e. by using the parsed
> refspecs returned by remote_get()) completely outdated.
> Second, while it properly frees those refspecs, i.e. the array and all
> its string fields, it will now leak the memory pointed by the
> 'refspec' variable.  However, why free just that one field of the
> 'struct *remote'?  Alas, we don't seem to have a free_remote()
> function...