logsink: fix multiple issues with LogSink::ToString() #852
Conversation
anpol
force-pushed
the
log-sink-to-string
branch
3 times, most recently
from
0642ff6
to
1695f91
Compare
Codecov Report
@@ Coverage Diff @@
## master #852 +/- ##
==========================================
+ Coverage 73.17% 73.21% +0.04%
==========================================
Files 17 17
Lines 3277 3282 +5
==========================================
+ Hits 2398 2403 +5
Misses 879 879
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
src/logging.cc
Outdated
| #ifdef _MSC_VER | ||
| #pragma push_macro("write") | ||
| #undef write | ||
| #endif // _MSC_VER |
There was a problem hiding this comment.
Can you elaborate why this is suddenly necessary? I'm not seeing any warnings issued by Github runners.
There was a problem hiding this comment.
The error was
'_write': is not a member of 'std::ostringstream
https://buildkite.com/bazel/google-logging/builds/2869#01829651-f13d-4665-9c42-dfd29c8ac36a
The source of the problem is
src/windows/port.h:87:#define write _writeThere was a problem hiding this comment.
Thanks. Instead of undefing the macro, you can prevent its expansion by protecting the write calls as follows:
(stream.write)(message, static_cast<std::streamsize>(message_len));Please also add a comment for the reason why macro expansion is prevented.
There was a problem hiding this comment.
OK, thank you. That would require redefining write macro with three parameters.
There was a problem hiding this comment.
Good point. I missed that.
anpol
force-pushed
the
log-sink-to-string
branch
from
1695f91
to
fa1a70d
Compare
1. Initializing std::ostringstream with a string makes no sense, as the string becomes an initial value of an underlying buffer; seek-to-end is not performed, so the initial value gets completely overwritten by subsequent writing. 2. Flag `log_year_in_prefix` should be considered, as if formatting a regular logging message. 3. Writing a buffer to std::ostream is better expressed with write(s,n).
anpol
force-pushed
the
log-sink-to-string
branch
from
fa1a70d
to
a1608cc
Compare
Initializing
std::ostringstreamwith a string makes no sense, as the string becomes an initial value of an underlying buffer; seek-to-end is not performed, so the initial value gets completely overwritten by subsequent writing.Flag
log_year_in_prefixshould be considered, as if formatting a regular logging message.Writing a buffer to
std::ostreamis better expressed withstream.write(s,n).