Re: [PATCH ps/stash-in-c] strbuf_vinsertf: provide the correct buffer size to vsnprintf

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Hannes,

On Mon, 4 Feb 2019, Johannes Sixt wrote:

> Am 04.02.19 um 11:38 schrieb Johannes Schindelin:
> > On Mon, 4 Feb 2019, Johannes Sixt wrote:
> > 
> >> Am 03.02.19 um 17:51 schrieb Johannes Sixt:
> >>> strbuf_vinsertf inserts a formatted string in the middle of an existing
> >>> strbuf value.
> >>
> >> Quite frankly, this is a really unusual operation, and I'd prefer to get
> >> rid of it. There is only one call, and it looks like it only wants to be
> >> lazy and save one strbuf variable.
> > 
> > The only reason why there are not more callers is that I did not convert
> > any of the appropriate places. We have quite a few places where we
> > allocate a new strbuf for the sole purpose of formatting something that is
> > then inserted into an already existing strbuf (possibly extending the
> > buffer, which might require a move of the buffer just because that
> > temporary strbuf is in the way).
> > 
> > It does not sound like good practice to me to allocate things left and
> > right, only to reallocate something that was just allocated anyway and to
> > copy things into that and then release things left and right.
> 
> I prefer separation of concerns at the expense of a bit of resource
> consumption that is not measurable. But that is the only argument that I
> have.

As far as I am concerned, the `strbuf_vinsertf()` function does not
conflate any concerns... It has one, very easily explained concern: to
interpolate text into the middle of a `strbuf`, much like `strbuf_vaddf()`
interpolates text at the end of one...

You found a bug in the implementation of that concern, is all, and fixed
it, for which I am grateful.

Ciao,
Dscho