[cmake-developers] [PATCH v3 5/7] For consoles output on Windows use our own std::streambuf

[Mike Gelfand]

Just a small nore: as I failed to mention this before, the approach
taken in this patch will lead to messages to be printed to stdout and
stderr in CMake's internal encoding (UTF-8?) when ConsoleBuf is not
used, i.e. 1) on non-Windows and 2) on Windows with one or another
stream redirected to file/pipe (considering corresponding check is in
place). Seems to be the case anyway now, but maybe something to think about.

On 07/06/2016 10:12 PM, Dāvis Mosāns wrote:
> --- a/Source/cmakemain.cxx
> +++ b/Source/cmakemain.cxx
> @@ -171,6 +189,16 @@ int main(int ac, char const* const* av)
>  #ifdef CMAKE_BUILD_WITH_CMAKE
>    cmDynamicLoader::FlushCache();
>  #endif
> +#if defined(_WIN32)
> +  if (cbufio) {
> +    delete cbufio;
> +    std::cout.rdbuf(coutbuf);
> +  }
> +  if (cbuferr) {
> +    delete cbuferr;
> +    std::cerr.rdbuf(cerrbuf);
> +  }
> +#endif
>    return ret;
>  }
>  
If exception was thrown in the beginning of main (as it seems you expect
one there), `coutbuf` and/or `cerrbuf` could be nullptr, and passing
nullptr here will fail.

> +  template<class CharT, class Traits = std::char_traits<CharT>>
C++11 in KWSys here as well (>> at the end of line is a single token in
pre-C++11)...

> +          throw std::system_error(::GetLastError(), std::system_category(), "GetStdHandle([snip]) returned INVALID_HANDLE_VALUE");
... and here (no std::system_error in pre-C++11)... and other places
further.

> +          m_hOutput = ::GetStdHandle(STD_ERROR_HANDLE);
Using WinAPI but missing corresponding system #include.

> [snip]
> +      virtual int sync() {
> [snip]
> +          const std::wstring &wBuffer = getBuffer(m_obuffer);
> [snip]
There are two `getBuffer`, one returning by value (for narrow char type)
and another by reference (for wide char type). When first one is in
effect, assigning the result to reference variable seems to be
guaranteed to work by the standard (at least I remember reading
something about it), but in reality it works in some cases and fails
(e.g. segfaults) in other, depending on compiler and flags.

> +        this->setg((char_type *)m_ibuffer.data(), (char_type *)m_ibuffer.data(), (char_type *)m_ibuffer.data() + m_ibuffer.size());
> +        this->setp((char_type *)m_obuffer.data(), (char_type *)m_obuffer.data() + m_obuffer.size());
This pattern over and over again. Needs to be put into separate member
function, otherwise some day someone will mix "i" and "o" and wonder why
doesn't it work.

Also, I don't see that many calls to set(g|p) in that SO snipped, are
they all really needed?

Regards,
Mike