On Wed, Mar 20, 2024, at 01:35, Jeff King wrote: > When pretty-printing a commit in the email format, we have to fill in > the "after subject" field of the pretty_print_context with any extra > headers the user provided (e.g., from "--to" or "--cc" options) plus any > special MIME headers. > > We return an out-pointer that sometimes points to a newly heap-allocated > string and sometimes not. To avoid leaking, we store the allocated > version in a buffer with static lifetime, which is ugly. Worse, as we > extend the header feature, we'll end up having to repeat this ugly > pattern. > > Instead, let's have our out-pointer pass ownership back to the caller, > and duplicate the string when necessary. This does mean one extra > allocation per commit when you use extra headers, but in the context of > format-patch which is showing diffs, I don't think that's even > measurable. > > Signed-off-by: Jeff King <peff@xxxxxxxx> Good presentation of motivation here. > --- > I don't think the extra allocation is a big deal, but if we do, there > are some other options: > > - instead of an out-pointer we could take a strbuf, and the caller > could reset and reuse a strbuf for each commit > > - the after_subject stuff could become a callback; we discussed this a > long time ago (I had no recollection of the thread until finding it > in the archive just now): > > > https://lore.kernel.org/git/20170325211149.yyvocmdfw4zbjyoi@xxxxxxxxxxxxxxxxxxxxx/ > > - this log_write_email_headers() function prints part of its output to > stdout, and shoves part of it into the after_subject field to be > shown by the pretty-printer. I wonder if it could just format the > subject itself (though that would make "rev-list --format=email" > even more awkward, I guess). I don’t quite understand all of these alternatives but the first one makes sense. Leave the responsibility to the caller. That could work. -- Kristoffer Haugsbakk
