2016-07-07 1:36 GMT+03:00 Mike Gelfand <mike...@mikedld.com>:
> On 07/07/2016 12:42 AM, clin...@elemtech.com wrote:
>> From what I remember, WriteConsoleW doesn't support redirection to a
>> file or pipe.  I don't see an alternative in the patch for the case
>> where stdout is not attached to the console.
> The SO thread I pointed at actually contained a check for whether
> corresponding stream is linked to terminal or not. Replacing stream
> buffers unconditionally is not right.
>

Sorry, I missed this... Will fix.

2016-07-07 2:21 GMT+03:00 Mike Gelfand <mike...@mikedld.com>:
> 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.
>

I know this, it always have been the case and with this patch I fix only
output to console because currently it would output internal encoding
which is unreadable for non-ASCII because console expects different codepage.

As for encoding used for files it doesn't matter because we use same one
for both writing and reading. It would matter if other applications would
want to read it. And because it's set as UTF-8 it should work fine.

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

coutbuf can be null only if

    cbufio = new cmsys::ConsoleBuf();

threw an exception in which case cbufio will be null and so it won't be set
here. And same for cerrbuf, it will be set if cbuferr is set in which case it
means there weren't any exception. While I think it's not needed, I can
add check for it and set rdbuf only when not null.

>
>> +        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?
>

There aren't really that many calls, we set both once in constructor,
both once in sync and setg twice (differently) in underflow, because
two cases:

"The function may update gptr, egptr and eback pointers to define the
location of newly loaded data (if any). On failure, the function ensures
that either gptr() == nullptr or gptr() == egptr"

from http://en.cppreference.com/w/cpp/io/basic_streambuf/underflow
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Reply via email to