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

Re: [PATCH] Allocate msg only after fatal checks to avoid leaks

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 15, 2025, 00:45 UTC
Message-ID
<xmqqh60h6ddy.fsf@gitster.g>
In-Reply-To
<20250614230158.GA2568638@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 7 quoted lines
> And while it may be tempting to say "well, it does not hurt to free them
> on the die() path", in my opinion that way madness lies. You may have
> access to some local variables that can be freed, but there will be many
> other heap allocations that you don't even know about! Here's a toy
> example from a similar discussion a few years ago:
>
>   https://lore.kernel.org/git/YNypPeoZTRiOxPPQ@coredump.intra.peff.net/

Yeah, I recall that discussion and the example. Yes, we should not have to crawl up from a direct caller of die() and free everything these stack frames hold.

Show 17 quoted lines
> I.e., this:
>
> diff --git a/builtin/notes.c b/builtin/notes.c
> index cc1163242f..f3d5eda104 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -321,12 +321,8 @@ static int parse_reuse_arg(const struct option *opt, const char *arg, int unset)
>  		die(_("failed to resolve '%s' as a valid ref."), arg);
>  	if (!(value = odb_read_object(the_repository->objects, &object, &type, &len)))
>  		die(_("failed to read object '%s'."), arg);
> -	if (type != OBJ_BLOB) {
> -		strbuf_release(&msg->buf);
> -		free(value);
> -		free(msg);
> +	if (type != OBJ_BLOB)
>  		die(_("cannot read note data from non-blob object '%s'."), arg);
> -	}
Much nicer.
Previous: Jeff King
Message 7 of 7 in “Allocate msg only after fatal checks to avoid leaks”
  1. Allocate msg only after fatal checks to avoid leaksAlex via GitGitGadget, Jun 13, 2025
  2. Junio C HamanoJun 13, 2025
  3. lidongyanJun 14, 2025
  4. Junio C HamanoJun 14, 2025
  5. lidongyanJun 14, 2025
  6. Jeff KingJun 14, 2025
  7. Junio C HamanoJun 15, 2025

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.