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