Re: [PATCH v2] Refactor recv_sideband()

From: Junio C Hamano <gitster@pobox.com>
To: Lukas Fleischer <lfleischer@lfos.de>
Cc: "Jeff King" <peff@peff.net>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Nicolas Pitre" <nico@fluxnic.net>,
	"Johannes Sixt" <j6t@kdbg.org>
Subject: Re: [PATCH v2] Refactor recv_sideband()
Date: Mon, 27 Jun 2016 22:20:39 -0700	[thread overview]
Message-ID: <xmqqy45panyw.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <146705966655.11886.6547584744094511110@typhoon> (Lukas Fleischer's message of "Mon, 27 Jun 2016 22:34:27 +0200")

Lukas Fleischer <lfleischer@lfos.de> writes:

> I do not see how using fwrite() buys us anything. Neither fwrite() nor
> fputs() nor fprintf() guarantee to call write() only once.

That is not the point.

Your first attempt split what used to be a single fprintf(), which
(as Nico explained) ordinarily will result in an atomic write as
long as you are feeding a reasonable size buf, into two calls,
_guaranteeing_ that it will _not_ be atomic.  That was a bad change.

By accumulating things in strbuf (instead of char[] with hand-rolled
counting logic like the original before your patch) and feeding the
result out to a single fprintf(stderr, "%s", buf.buf), you will not
be making anything _worse_ compared to the code before your patch.
At the same time, you _are_ making the logic to accumulate the
output to a buffer far easier to read, which would be a net plus.