Re: [PATCH] mingw: make stderr unbuffered again

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hannes,

On Tue, 14 Feb 2017, Johannes Sixt wrote:

> Am 14.02.2017 um 15:47 schrieb Johannes Schindelin:
> > On Mon, 13 Feb 2017, Junio C Hamano wrote:
> > > Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> > > > When removing the hack for isatty(), we actually removed more than
> > > > just an isatty() hack: we removed the hack where internal data
> > > > structures of the MSVC runtime are modified in order to redirect
> > > > stdout/stderr.
> > > >
> > > > Instead of using that hack (that does not work with newer versions
> > > > of the runtime, anyway), we replaced it by reopening the
> > > > respective file descriptors.
> > > >
> > > > What we forgot was to mark stderr as unbuffered again.
> 
> I do not see how the earlier patch turned stderr from unbuffered to
> buffered, as it did not add or remove any setvbuf() call. Can you
> explain?

Certainly. I hoped that the commit message would do the job, but then, I
am under time pressure these days, so it was probably a bit terse.

The hack we used to make isatty() work used to change data structures
directly that are *internal* to the MSVC runtime. That is, instead of
*really* redirecting stdout/stderr, we simply changed the target of the
existing stdout/stderr and thereby could fool MSVC into believing that it
is *still* writing to the terminal (because the bit is set) and that it is
*not* a pipe (because we forcibly unset that bit).

Needless to say, this meddling with internal data structures is not only
prone to break with future updates of the MSVC runtime, it is also
inappropriate because the implementation may rely on certain side effects
(or not so side effects) that may very well cause crashes or data loss.
Imagine, for example, that the internal data structure were variable-size,
based on the HANDLE type. That is totally legitimate for an internal data
structure. And if we meddle with the HANDLE, we can easily cause data
corruption.

As GCC is basically tied to using an older version of the MSVC runtime
(unlike GLIBC, multiple versions of the MSVC runtime can coexist happily,
making it relatively easy to maintain backwards-compatibility, but that
concept is foreign to GCC), this used to not be a problem.

However, with our recent push to allow developing, building, debugging and
performance profiling in Visual Studio, that limitation no longer holds
true: if you develop with Visual Studio 2015, you will link to a newer
MSVC runtime, and that runtime changed those internal data structures
rather dramatically.

That means that we had to come up with a non-hacky way to redirect
stdout/stderr (e.g. to parse ESC color sequences and apply them to the
Win32 Console as appropriate) and still have isatty() report what we want
it to report. That is, if we redirect stdout/stderr to a pipe that parses
the color sequences for use in the Win32 Console, we want isatty() to
report that we are writing to a Terminal Typewriter (a charming
anachronism, isn't it?).

My colleague Jeff Hostetler worked on this and figured out that the only
way to do this properly is to wrap isatty() and override it via a #define,
and simply remember when stdout/stderr referred to a terminal before
redirecting, say, to that pipe.

As my famously broken patch to override isatty() (so that /dev/null would
not be treated as a terminal) *already* overrode isatty() with a custom
wrapper, and as it was broken and needed fixing, I decided to reconcile
the two approaches into what is now the version in `master`.

So instead of "bending" the target HANDLE of the existing stdout/stderr
(which would *naturally* have kept the buffered/unbuffered nature as-is),
we now redirect with correct API calls. And the patch I provided at the
bottom of this mail thread reinstates the unbuffered nature of stderr now
that it gets reopened.

Hopefully that makes it clear why the setvbuf() call is required now, but
was previously unnecessary?

Ciao,
Dscho



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]