git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

Previous: Johannes SixtNext: Johannes Schindelin
Message 5 of 13 in “strbuf_vinsertf: provide the correct buffer size to vsnprintf”
  1. strbuf_vinsertf: provide the correct buffer size to vsnprintfJohannes Sixt, Feb 3, 2019
  2. Johannes SixtFeb 4, 2019
  3. Johannes SchindelinFeb 4, 2019
  4. Johannes SixtFeb 4, 2019
  5. Johannes SchindelinFeb 5, 2019
  6. Johannes SchindelinFeb 4, 2019
  7. Junio C HamanoFeb 4, 2019
  8. Johannes SchindelinFeb 5, 2019
  9. Junio C HamanoFeb 5, 2019
  10. Johannes SchindelinFeb 6, 2019
  11. Johannes SixtFeb 5, 2019
  12. Johannes SchindelinFeb 6, 2019
  13. Johannes SixtFeb 6, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.