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 6, 2019, 11:56 UTC
Message-ID
<nycvar.QRO.7.76.6.1902061242380.41@tvgsbejvaqbjf.bet>
In-Reply-To
<ef5f66fb-7467-93be-00fd-7900ce961f72@kdbg.org>
Hi Hannes,
On Tue, 5 Feb 2019, Johannes Sixt wrote:
Show 11 quoted lines
> Am 05.02.19 um 12:06 schrieb Johannes Schindelin:
> > The real examples are much more mundane, and very different from what you
> > suspected, e.g. inserting the tag header before the tag message in
> > `create_tag()` in `builtin/tag.c`. Basically, it is building up a strbuf
> > for the sake of calling `strbuf_insert()` to insert that string elsewhere.
> 
> I had a look, and this example does not convince me at all. If
> separation of concerns were applied well around the launch_editor APIs,
> you would not need strbuf_insert() in the first place. But, alas, these
> functions focus more on DRY, and that is often the opposite of
> separation of concerns.
So you actually are convinced that it is needed in this instance. Good.
Show 9 quoted lines
> > In any case, the mere existence, and use, of `strbuf_insert()` is a
> > pretty clear counter case to the idea that `strbuf_vinsertf()` would
> > encourage invalid code flows.
> 
> Nobody wants to get rid of strbuf_insert(). I have worked with at least
> 3 string APIs. All have insert operations, and some have string
> formatting capabilities, but none of them conflate the two operations
> into one. Maybe, there is a plan behind it? strbuf is the first (my 4th)
> API that does, and it was non-trivial to get it right...

Sorry, by that token `strbuf_vaddf()` would already be a violation of the separation of concerns: it also makes space first, and then it also formats a string.

Render me unconvinced.

I am still glad you found and fixed the bug, Dscho

Previous: Johannes SixtNext: Johannes Sixt
Message 12 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.