Re: [PATCH v2 1/3] clone: remove double bundle list clear code

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

 



Toon Claes <toon@xxxxxxxxx> writes:

> The bundle list transport->bundles is filled by
> transport_get_remote_bundle_uri(). Only when the list is not used, it is
> cleared right away by calling clear_bundle_list().
>
> This looks like we leak memory allocated for the list when
> transport->bundles *is* used. But in fact, transport->bundles is cleaned
> up in transport_disconnect() near the end of cmd_clone().
>
> Remove the double clean up of transport->bundles, and depend solely on
> transport_disconnect() to take care of it.
>
> Also add a test case that hits this code, but due to other leaks we
> cannot mark it as leak-free.
>
> Signed-off-by: Toon Claes <toon@xxxxxxxxx>
> ---
>  builtin/clone.c             |  3 ---
>  t/t5558-clone-bundle-uri.sh | 28 +++++++++++++++++++++++++++-
>  2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index af6017d41a..aa507395a0 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -1419,9 +1419,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  			else if (fetch_bundle_list(the_repository,
>  						   transport->bundles))
>  				warning(_("failed to fetch advertised bundles"));
> -		} else {
> -			clear_bundle_list(transport->bundles);
> -			FREE_AND_NULL(transport->bundles);
>

There is a small difference that here we also set `transport->bundles`
to NULL, whereas in `transport_disconnect()` we only do `free(...)`. I
don't see any issues because of it though. This cleanup looks good.

[snip]

Attachment: signature.asc
Description: PGP signature


[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]

  Powered by Linux