All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Sixt <j6t@kdbg.org>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH ps/stash-in-c] strbuf_vinsertf: provide the correct buffer size to vsnprintf
Date: Mon, 4 Feb 2019 22:13:13 +0100	[thread overview]
Message-ID: <ea7b1875-1a23-e4e8-e30e-5b769d282323@kdbg.org> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.1902041135320.41@tvgsbejvaqbjf.bet>

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.

-- Hannes

  reply	other threads:[~2019-02-04 21:13 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-03 16:51 [PATCH ps/stash-in-c] strbuf_vinsertf: provide the correct buffer size to vsnprintf Johannes Sixt
2019-02-04  7:25 ` Johannes Sixt
2019-02-04 10:38   ` Johannes Schindelin
2019-02-04 21:13     ` Johannes Sixt [this message]
2019-02-05 10:38       ` Johannes Schindelin
2019-02-04 10:54 ` Johannes Schindelin
2019-02-04 21:57 ` Junio C Hamano
2019-02-05 11:06   ` Johannes Schindelin
2019-02-05 18:01     ` Junio C Hamano
2019-02-06 11:41       ` Johannes Schindelin
2019-02-05 20:30     ` Johannes Sixt
2019-02-06 11:56       ` Johannes Schindelin
2019-02-06 18:11         ` Johannes Sixt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ea7b1875-1a23-e4e8-e30e-5b769d282323@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=ungureanupaulsebastian@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.