SZEDER Gábor <szeder.dev@xxxxxxxxx> writes: > On Mon, May 15, 2017 at 1:05 PM, SZEDER Gábor <szeder.dev@xxxxxxxxx> wrote: >> From: Jeff King <peff@xxxxxxxx> >> >> 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@xxxxxxxx> >> Signed-off-by: SZEDER Gábor <szeder.dev@xxxxxxxxx> >> --- >> 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... I was sifting entries in the draft "What's cooking" report to find topics to merge to 'next'. I read the series over and as Peff said in his <20170515224615.f6hnnfngwpierqk4@xxxxxxxxxxxxxxxxxxxxx>, I think the series overall is good. Do you want to update the proposed log message for this one before the entire thing is merged to 'next'?