git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Karthik Nayak <karthik.188@gmail.com>
Date
Jul 26, 2024, 08:51 UTC
Message-ID
<CAOLa=ZRJDoy=n=ZTULc9rBT=mQPyWRUBW7E5-JeQsPUYaDV0=Q@mail.gmail.com>
In-Reply-To
<20240724144957.3033840-2-toon@iotcl.com>
Toon Claes <toon@iotcl.com> writes:
Show 32 quoted lines
> 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@iotcl.com>
> ---
>  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]
Previous: Toon ClaesNext: Justin Tobler
Message 5 of 17 in “fetch: use bundle URIs when having creationToken heuristic”
  1. fetch: use bundle URIs when having creationToken heuristicToon Claes, Jul 22, 2024
  2. Junio C HamanoJul 22, 2024
  3. 0/3 fetch: use bundle URIs when having creationToken heuristicToon Claes, Jul 24, 2024
  4. 1/3 clone: remove double bundle list clear codeToon Claes, Jul 24, 2024
  5. Karthik NayakJul 26, 2024
  6. Justin ToblerJul 26, 2024
  7. Toon claesAug 2, 2024
  8. 2/3 transport: introduce transport_has_remote_bundle_uri()Toon Claes, Jul 24, 2024
  9. Karthik NayakJul 26, 2024
  10. Junio C HamanoJul 26, 2024
  11. 3/3 fetch: use bundle URIs when having creationToken heuristicToon Claes, Jul 24, 2024
  12. Karthik NayakJul 26, 2024
  13. Patrick SteinhardtJul 26, 2024
  14. Toon claesAug 2, 2024
  15. Patrick SteinhardtAug 22, 2024
  16. Toon ClaesSep 27, 2024
  17. builtin/clone: teach git-clone(1) the --ref= argumentToon Claes, Sep 27, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.