Re: [PATCH v2] Refactor recv_sideband()

[Lukas Fleischer <lfleischer@lfos.de>]

On Mon, 27 Jun 2016 at 19:50:13, Junio C Hamano wrote:
> 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.

> >
> > 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.