On Tue, Mar 19, 2024 at 07:35:36PM +0100, Kristoffer Haugsbakk wrote:
> Add `pe_header` to `rev_info` to store per-email headers.
It is only just now that I realized that "pe" stands for per-email
(though to be fair I was not really focused on the intent of the series
when reading v1). Can we just call it per_email_headers or something?
> The next commit will add an option to `format-patch` which will allow
> the user to store headers per-email; a complement to options like
> `--add-header`.
>
> To make this possible we need a new field to store these headers. We
> also need to take ownership of `extra_headers_p` in
> `log_write_email_headers`; facilitate this by removing constness from
> the relevant pointers.
There are three pointers at play here:
- ctx.after_subject has its const removed, since it will now always be
allocated by log_write_email_headers(), and then freed by the
caller. Makes sense Though it looks like we only free in show_log(),
and the free in make_cover_letter() is not added until patch 2?
- rev_info.extra_headers has its const removed here, but I don't think
that is helping anything. We only use it to write into the "headers"
strbuf in log_write_email_headers(), which always returns
headers.buf (or NULL).
- rev.pe_headers is introduced as non-const because it is allocated
and freed for each email. That makes some sense, though if we
followed the pattern of rev.extra_headers, then the pointer is
conceptually "const" within the rev_info struct, and it is the
caller who keeps track of the allocation (using a to_free variable).
Possibly we should do the same here?
I do still think this could be split in a more obvious way, leaving the
pe_headers bits until they are actually needed. Let me see if I can
sketch it up.
-Peff
