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

Re: [PATCH 5/6] format-patch: return an allocated string from log_write_email_headers()

From
Kristoffer Haugsbakk <code@khaugsbakk.name>
Date
Mar 22, 2024, 22:06 UTC
Message-ID
<c94e0ba2-87b9-4272-afce-67f5faf0275f@app.fastmail.com>
In-Reply-To
<20240320003533.GE904136@coredump.intra.peff.net>
On Wed, Mar 20, 2024, at 01:35, Jeff King wrote:
Show 18 quoted lines
> 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@peff.net>
Good presentation of motivation here.
Show 19 quoted lines
> ---
> 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@sigill.intra.peff.net/
>
>   - 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
Previous: Jeff KingNext: Jeff King
Message 24 of 34 in “format-patch: teach `--header-cmd`”
  1. 0/3 format-patch: teach `--header-cmd`Kristoffer Haugsbakk, Mar 7, 2024
  2. 1/3 log-tree: take ownership of pointerKristoffer Haugsbakk, Mar 7, 2024
  3. Jeff KingMar 12, 2024
  4. Kristoffer HaugsbakkMar 12, 2024
  5. Jeff KingMar 13, 2024
  6. Kristoffer HaugsbakkMar 13, 2024
  7. 2/3 format-patch: teach `--header-cmd`Kristoffer Haugsbakk, Mar 7, 2024
  8. Kristoffer HaugsbakkMar 8, 2024
  9. Jean-Noël AvilaMar 11, 2024
  10. Kristoffer HaugsbakkMar 12, 2024
  11. 3/3 format-patch: check if header output looks validKristoffer Haugsbakk, Mar 7, 2024
  12. 0/3 format-patch: teach `--header-cmd`Kristoffer Haugsbakk, Mar 19, 2024
  13. 1/3 revision: add a per-email field to rev-infoKristoffer Haugsbakk, Mar 19, 2024
  14. Jeff KingMar 19, 2024
  15. Kristoffer HaugsbakkMar 19, 2024
  16. Jeff KingMar 20, 2024
  17. 1/6 shortlog: stop setting pp.print_email_subjectJeff King, Mar 20, 2024
  18. 2/6 pretty: split oneline and email subject printingJeff King, Mar 20, 2024
  19. Kristoffer HaugsbakkMar 22, 2024
  20. 3/6 pretty: drop print_email_subject flagJeff King, Mar 20, 2024
  21. 4/6 log: do not set up extra_headers for non-email formatsJeff King, Mar 20, 2024
  22. Kristoffer HaugsbakkMar 22, 2024
  23. 5/6 format-patch: return an allocated string from log_write_email_headers()Jeff King, Mar 20, 2024
  24. Kristoffer HaugsbakkMar 22, 2024
  25. 6/6 format-patch: simplify after-subject MIME header handlingJeff King, Mar 20, 2024
  26. Kristoffer HaugsbakkMar 22, 2024
  27. Jeff KingMar 20, 2024
  28. Kristoffer HaugsbakkMar 22, 2024
  29. 7/6 format-patch: fix leak of empty header stringJeff King, Mar 22, 2024
  30. Kristoffer HaugsbakkMar 22, 2024
  31. Junio C HamanoMar 22, 2024
  32. Kristoffer HaugsbakkMar 22, 2024
  33. 2/3 format-patch: teach `--header-cmd`Kristoffer Haugsbakk, Mar 19, 2024
  34. 3/3 format-patch: check if header output looks validKristoffer Haugsbakk, Mar 19, 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.