Re: [PATCH ps/stash-in-c] strbuf_vinsertf: provide the correct buffer size to vsnprintf
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Feb 5, 2019, 10:38 UTC
- Message-ID
- <nycvar.QRO.7.76.6.1902051136410.41@tvgsbejvaqbjf.bet>
- In-Reply-To
- <ea7b1875-1a23-e4e8-e30e-5b769d282323@kdbg.org>
Hi Hannes,
On Mon, 4 Feb 2019, Johannes Sixt wrote:
Show 25 quoted lines
> 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