-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added synchronization with the main storage device before direct recording #3668
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR but please revert unrelated formatting changes and add more context. I applied clang-format separately.
c999ce9
to
7c19f40
Compare
Done. |
include/fmt/ostream.h
Outdated
#endif | ||
return false; | ||
if (!c_file) return false; | ||
os.flush(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should only flush the buffer when writing to a console. I suggest moving the ostream change to a separate PR and addressing this issue. The rest LGTM.
7c19f40
to
bb7490e
Compare
bb7490e
to
4fe961f
Compare
Merged without the ostream part in 3b7f58a. Thanks. |
if (!_isatty(fd)) return false; | ||
std::fflush(f); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding fflush
here is not correct. This function write_console()
is called either from the C stream print() or the C++ iostream print(). In the latter case, fflush
should not be called at all, instead the ostream flush should be invoked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This flush is indeed redundant for iostream but in the future please report issues like these once to avoid too much notification noise.
Added reset of caching stream buffers before direct writing to the descriptor