Re: [PATCH v2] Refactor recv_sideband()

From: Lukas Fleischer <lfleischer@lfos.de>
Date: Jun 27, 2016, 20:34 UTC
Message-ID: <146705966655.11886.6547584744094511110@typhoon>
In-Reply-To: <xmqqbn2mbjxm.fsf@gitster.mtv.corp.google.com>

On Mon, 27 Jun 2016 at 19:50:13, Junio C Hamano wrote:

Show 7 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > On Mon, Jun 27, 2016 at 08:54:22AM -0700, Junio C Hamano wrote:
> >
> >> It's just you used xwrite() there that introduced a different issue.
> >> Wouldn't replacing it with fwrite(stderr) without changing anything
> >> else solve that?

I do not see how using fwrite() buys us anything. Neither fwrite() nor fputs() nor fprintf() guarantee to call write() only once. Each of these three functions is buffered when printing to stdout and unbuffered when printing to stderr. I do not think there is any serious implementation of any of those functions that performs segmented write() calls (but I might be mistaken). According to POSIX, write() can take up to SSIZE_MAX bytes which is guaranteed to be at least 32767 but is actually much larger on most systems (2^32 - 1 here). It is very unlikely that this limit will ever be reached by a single line of a diagnostic error message.

Frankly, there is a small benefit to fwrite() because we already know the string length from the strbuf and most fputs() implementations probably do something equivalent to

    fwrite(s, 1, strlen(s), stream);

I can switch to using fwrite() instead of fputs() in v4 if you prefer that.

Show 9 quoted lines

> >
> > I am having trouble actually seeing how the ANSI-emulation code gets
> > triggered, but the comment in color.h implies that it is only printf,
> > fprintf, and fputs that have the desired effect. So fwrite() may not be
> > sufficient, and we may need fprintf("%.*s", len, buf) or something.
> 
> I have no idea how, either X-<.  But you're probably right about the
> magic being limited to the printf family of functions---I do recall
> hearing something like that in the past.

I do not know anything about the emulation code as well but from a cursory read of winansi_init(), it looks like there is some magic that hooks into the stdout and stderr streams, redirects them to a named pipe, then replaces ANSI control codes and actually prints to the console from console_thread(). So it should work with any of the stream-based functions but not with write(), puts(), etc.