From: Phillip Wood Date: Mon, 09 Mar 2026 10:36:53 GMT Subject: Re: [PATCH v8 1/6] interpret-trailers: factor trailer rewriting Message-ID: In-Reply-To: On 06/03/2026 21:04, Junio C Hamano wrote: > Phillip Wood writes: > >> From: Li Chen >> >> Extract the trailer rewriting logic into a helper that appends to an >> output strbuf. >> >> Update interpret_trailers() to handle file I/O only: read input once, >> call the helper, and write the buffered result. >> >> This separation makes it easier to move the helper into trailer.c in the >> next commit. >> >> Signed-off-by: Phillip Wood >> Signed-off-by: Li Chen >> --- > > If "Li wrote it and signed it off, Phillip is relaying" is what is > happening, the sign-offs are in reverse order. If "Li incorporated > what Phillip wrote earlier in his larger work", then the sign-offs > may be fine, but it would be necessary to hint that that is what > happened in the proposed log message. I cannot quite tell which is > the case. It's the latter, I posted a diff[1] and Li based this patch on that diff Thanks Phillip [1] https://lore.kernel.org/git/7d12b046-365f-441c-af8e-8a39d61efbbd@gmail.com/ >> builtin/interpret-trailers.c | 57 ++++++++++++++++++++---------------- >> 1 file changed, 32 insertions(+), 25 deletions(-) >> >> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c >> index 41b0750e5af..69f9d67ec0e 100644 >> --- a/builtin/interpret-trailers.c >> +++ b/builtin/interpret-trailers.c >> @@ -136,32 +136,21 @@ static void read_input_file(struct strbuf *sb, const char *file) >> strbuf_complete_line(sb); >> } >> >> -static void interpret_trailers(const struct process_trailer_options *opts, >> - struct list_head *new_trailer_head, >> - const char *file) >> +static void process_trailers(const struct process_trailer_options *opts, >> + struct list_head *new_trailer_head, >> + struct strbuf *input, struct strbuf *out) >> { >> LIST_HEAD(head); >> - struct strbuf sb = STRBUF_INIT; >> - struct strbuf trailer_block_sb = STRBUF_INIT; >> struct trailer_block *trailer_block; >> - FILE *outfile = stdout; >> - >> - trailer_config_init(); >> - >> - read_input_file(&sb, file); >> - >> - if (opts->in_place) >> - outfile = create_in_place_tempfile(file); >> - >> - trailer_block = parse_trailers(opts, sb.buf, &head); >> + >> + trailer_block = parse_trailers(opts, input->buf, &head); >> >> /* Print the lines before the trailer block */ >> if (!opts->only_trailers) >> - fwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile); >> + strbuf_add(out, input->buf, trailer_block_start(trailer_block)); >> >> if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block)) >> - fprintf(outfile, "\n"); >> - >> + strbuf_addch(out, '\n'); >> >> if (!opts->only_input) { >> LIST_HEAD(config_head); >> @@ -173,22 +162,40 @@ static void interpret_trailers(const struct process_trailer_options *opts, >> } >> >> /* Print trailer block. */ >> - format_trailers(opts, &head, &trailer_block_sb); >> + format_trailers(opts, &head, out); >> free_trailers(&head); >> - fwrite(trailer_block_sb.buf, 1, trailer_block_sb.len, outfile); >> - strbuf_release(&trailer_block_sb); >> >> /* Print the lines after the trailer block as is. */ >> if (!opts->only_trailers) >> - fwrite(sb.buf + trailer_block_end(trailer_block), 1, >> - sb.len - trailer_block_end(trailer_block), outfile); >> + strbuf_add(out, input->buf + trailer_block_end(trailer_block), >> + input->len - trailer_block_end(trailer_block)); >> trailer_block_release(trailer_block); >> - >> +} >> + >> +static void interpret_trailers(const struct process_trailer_options *opts, >> + struct list_head *new_trailer_head, >> + const char *file) >> +{ >> + struct strbuf input = STRBUF_INIT; >> + struct strbuf out = STRBUF_INIT; >> + FILE *outfile = stdout; >> + >> + trailer_config_init(); >> + >> + read_input_file(&input, file); >> + >> + if (opts->in_place) >> + outfile = create_in_place_tempfile(file); >> + >> + process_trailers(opts, new_trailer_head, &input, &out); >> + >> + strbuf_write(&out, outfile); >> if (opts->in_place) >> if (rename_tempfile(&trailers_tempfile, file)) >> die_errno(_("could not rename temporary file to %s"), file); >> >> - strbuf_release(&sb); >> + strbuf_release(&input); >> + strbuf_release(&out); >> } >> >> int cmd_interpret_trailers(int argc,