Jeff King <peff@xxxxxxxx> writes: > 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 ;-) > 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.
