Re: [PATCH v5 1/3] notes.c: cleanup 'strbuf_grow' call in 'append_edit'
Junio C Hamano <gitster@pobox.com> wrote on Thu, 16 Feb 2023 10:39:05 -0800:
Show 7 quoted lines
> > Best guess may be that the author originally inserted "\n" manually by
> > direct manipulation of the strbuf rather than employing a strbuf
> > function, but then switched to strbuf_insert() before submitting the
> > series and forgot to remove the now-unnecessary strbuf_grow().
>
> Please do not speculate when you do not have to. "git blame" is
> your friend.
My bad, I checked with "git blame" but did not describe in detail.
Show 15 quoted lines
> 348f199b (builtin-notes: Refactor handling of -F option to allow
> combining -m and -F, 2010-02-13) added these to mimic the code
> introduced by 2347fae5 (builtin-notes: Add "append" subcommand for
> appending to note objects, 2010-02-13) that reads in previous note
> before the message. And the resulting code with explicit sizing is
> carried to this day.
>
> In the context of reading an existing note in, exact sizing may have
> made sense, but because the resulting note needs cleansing with
> stripspace() when appending with this option, such an exact sizing
> does not buy us all that much in practice.
>
> It may help avoiding overallocation due to ALLOC_GROW() slop, but
> nobody can feed so many long messages for it to matter from the
> command line.
Will apply by your content, thanks very much.