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

Re: [PATCH 2/2] format-patch: free elements of rev.ref_message_ids list

From
Junio C Hamano <gitster@pobox.com>
Date
May 19, 2023, 01:32 UTC
Message-ID
<xmqqttw9qf6p.fsf@gitster.g>
In-Reply-To
<20230519000543.GB1975194@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 22 quoted lines
> When we are showing multiple patches with format-patch, we have to
> repeatedly overwrite the rev.message_id field. We take care to avoid
> leaking the old value by either freeing it, or adding it to
> ref_message_ids, a string list of ids to reference in subsequent
> messages.
>
> But unfortunately we do leak the value via that string list. We try
> to clear the string list, courtesy of 89f45cf4eb (format-patch: don't
> leak "extra_headers" or "ref_message_ids", 2022-04-13). But since it was
> initialized as "nodup", the string list doesn't realize it owns the
> strings, and it leaks them.
>
> We have two options here:
>
>   1. Continue to init with "nodup", but then tweak the value of
>      ref_message_ids.strdup_strings just before clearing.
>
>   2. Init with "dup", but use "append_nodup" when transferring ownership
>      of strings to the list. Clearing just works.
>
> I picked the second here, as I think it calls attention to the tricky
> part (transferring ownership via the nodup call).

This is pretty much what I had in mind wheh I wrote the log message for the follow-up "Yikes, for now let's mark the test no longer sanitizer clean" patch. Very pleased to see a clean-up I punted materialize while I was looking the other way ;-)

Show 5 quoted lines
> There's one other related fix we have to do, though. We also insert the
> result of clean_message_id() into the list. This _sometimes_ allocates
> and sometimes does not, depending on whether we have to remove cruft
> from the end of the string. Let's teach it to consistently return an
> allocated string, so that the caller knows it must be freed.
Yes!
Thanks.
Previous: Jeff KingNext: Konstantin Khomoutov
Message 4 of 6 in “a few format-patch leak fixes”
  1. 0/2 a few format-patch leak fixesJeff King, May 19, 2023
  2. 1/2 format-patch: free rev.message_id when exitingJeff King, May 19, 2023
  3. 2/2 format-patch: free elements of rev.ref_message_ids listJeff King, May 19, 2023
  4. Junio C HamanoMay 19, 2023
  5. Konstantin KhomoutovMay 19, 2023
  6. Junio C HamanoMay 19, 2023

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.