Hi Phillip,
---- On Tue, 11 Nov 2025 00:27:38 +0800 Phillip Wood <phillip.wood123@gmail.com> wrote ---
> On 05/11/2025 16:57, Junio C Hamano wrote:
> > Li Chen <me@linux.beauty> writes:
> >
> >> From: Li Chen <chenl311@chinatelecom.cn>
> >>
> >> Extracted trailer processing into a helper that accumulates output in
> >> a strbuf before writing.
> >>
> >> Updated interpret_trailers() to reuse the helper, buffer output, and
> >> clean up both input and output buffers after writing.
> >
> > Imperative?
> >
> >>
> >> Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
> >> ---
> >> builtin/interpret-trailers.c | 51 ++++++++++++++++++++----------------
> >> 1 file changed, 29 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
> >> index 41b0750e5a..4c90580fff 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 *sb, struct strbuf *out)
> >
> > So we gained *out strbuf; in the preimage below I see fwrite(),
> > fprintf(), etc. to outfile that is either stdout or tempfile, but
> > presumably the output all will be captured in the strbuf instead,
> > which makes sense. It is a bit curious what the new paramater sb
> > is, but this is a file-scope static helper, so it does not strictly
> > require documenting. Having a comment would still be nicer, though,
> > unlike "struct process_trailer_options" that is very limited
> > purpose, "strbuf" can be used for any string processing, so a good
> > variable name like "out" that conveys what it is used for by
> > implication is good, but "sb", which is obvious abbreviation for
> > "Str Buf", conveys no useful information.
>
> This patch is based on my suggestion[1]. I had intended to rename "sb"
> to "in" but forgot to do so before posting that diff. Here's my signoff
> which Li should add before their own
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
I'm sorry that your signoff is missing; I will add it in the next version.
Regards,
Li
Li Chen <me@linux.beauty>