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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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...




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]