From: Phillip Wood Date: Mon, 02 Mar 2026 14:56:04 GMT Subject: Re: [PATCH v7 1/5] interpret-trailers: factor trailer rewriting Message-ID: <56b1b6b6-94c2-4a2f-b473-9b4d09d6f52e@gmail.com> In-Reply-To: <20260224070552.148591-2-me@linux.beauty> Hi Li On 24/02/2026 07:05, Li Chen wrote: > 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. This is still missing my sign off c.f. https://lore.kernel.org/f5152523-f7ff-4dee-a685-fb0b74cd6a56@gmail.com > Signed-off-by: Li Chen > --- > v7: > Use strbuf_write() when emitting buffered output. Also renamed "sb" to "input" Apart from the missing sign off this all looks good. Thanks Phillip > > builtin/interpret-trailers.c | 53 ++++++++++++++++++++---------------- > 1 file changed, 30 insertions(+), 23 deletions(-) > > diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c > index 41b0750e5a..69f9d67ec0 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,