diff --git a/CMakeLists.txt b/CMakeLists.txt index 8f6ca16d..80248c9d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -278,6 +278,18 @@ if(CMAKE_GENERATOR MATCHES "Visual Studio") source_group(sources FILES ${VERSION_RC}) endif() +# --------------------------------------------------------------------------------------- +# Check if fwrite_unlocked/_fwrite_nolock is available +# --------------------------------------------------------------------------------------- +include(CheckSymbolExists) +if(WIN32) + check_symbol_exists(_fwrite_nolock "stdio.h" HAVE_FWRITE_UNLOCKED) +else () + check_symbol_exists(fwrite_unlocked "stdio.h" HAVE_FWRITE_UNLOCKED) +endif() +if(HAVE_FWRITE_UNLOCKED) + target_compile_definitions(spdlog PRIVATE SPDLOG_FWRITE_UNLOCKED) +endif() # --------------------------------------------------------------------------------------- # Add required libraries for Android CMake build # --------------------------------------------------------------------------------------- diff --git a/include/spdlog/details/os.h b/include/spdlog/details/os.h index 83dfac7f..d23c57fc 100644 --- a/include/spdlog/details/os.h +++ b/include/spdlog/details/os.h @@ -100,6 +100,11 @@ SPDLOG_API std::string getenv(const char *field); // Return true on success. SPDLOG_API bool fsync(FILE *fp); + +// Do non-locking fwrite if possible by the os or use the regular locking fwrite +// Return true on success. +SPDLOG_API bool fwrite_bytes(const void *ptr, const size_t n_bytes, FILE *fp); + } // namespace os } // namespace details } // namespace spdlog diff --git a/src/details/file_helper.cpp b/src/details/file_helper.cpp index e22bf5ed..6903ef85 100644 --- a/src/details/file_helper.cpp +++ b/src/details/file_helper.cpp @@ -93,7 +93,7 @@ void file_helper::write(const memory_buf_t &buf) const { if (fd_ == nullptr) return; const size_t msg_size = buf.size(); const auto *data = buf.data(); - if (std::fwrite(data, 1, msg_size, fd_) != msg_size) { + if (!os::fwrite_bytes(data, msg_size, fd_)) { throw_spdlog_ex("Failed writing to file " + os::filename_to_str(filename_), errno); } } diff --git a/src/details/os_unix.cpp b/src/details/os_unix.cpp index f91fc69b..089e81e0 100644 --- a/src/details/os_unix.cpp +++ b/src/details/os_unix.cpp @@ -326,6 +326,17 @@ std::string getenv(const char *field) { // Return true on success bool fsync(FILE *fp) { return ::fsync(fileno(fp)) == 0; } + +// Non locking ::fwrite if possible (SPDLOG_FWRITE_UNLOCKED defined) or use the regular locking fwrite +bool fwrite_bytes(const void *ptr, const size_t n_bytes, FILE *fp) +{ + #if defined(SPDLOG_FWRITE_UNLOCKED) + return ::fwrite_unlocked(ptr, 1, n_bytes, fp) == n_bytes; + #else + return std::fwrite(ptr, 1, n_bytes, fp) == n_bytes; + #endif +} + } // namespace os } // namespace details } // namespace spdlog diff --git a/src/details/os_windows.cpp b/src/details/os_windows.cpp index 6cbf94f3..1e75065b 100644 --- a/src/details/os_windows.cpp +++ b/src/details/os_windows.cpp @@ -304,6 +304,15 @@ std::string getenv(const char *field) { // Return true on success bool fsync(FILE *fp) { return FlushFileBuffers(reinterpret_cast(_get_osfhandle(_fileno(fp)))) != 0; } +// Non locking fwrite if possible (SPDLOG_FWRITE_UNLOCKED defined) or use the regular locking fwrite +bool fwrite_bytes(const void *ptr, const size_t n_bytes, FILE *fp) +{ + #if defined(SPDLOG_FWRITE_UNLOCKED) + return _fwrite_nolock(ptr, 1, n_bytes, fp) == n_bytes; + #else + return std::fwrite(ptr, 1, n_bytes, fp) == n_bytes; + #endif +} } // namespace os } // namespace details } // namespace spdlog diff --git a/src/sinks/ansicolor_sink.cpp b/src/sinks/ansicolor_sink.cpp index ecac32a9..d0bce481 100644 --- a/src/sinks/ansicolor_sink.cpp +++ b/src/sinks/ansicolor_sink.cpp @@ -87,12 +87,12 @@ void ansicolor_sink::flush_() { template void ansicolor_sink::print_ccode_(const string_view_t color_code) { - fwrite(color_code.data(), sizeof(char), color_code.size(), target_file_); + details::os::fwrite_bytes(color_code.data(), color_code.size(), target_file_); } template void ansicolor_sink::print_range_(const memory_buf_t &formatted, size_t start, size_t end) { - fwrite(formatted.data() + start, sizeof(char), end - start, target_file_); + details::os::fwrite_bytes(formatted.data() + start, end - start, target_file_); } template diff --git a/src/sinks/stdout_sinks.cpp b/src/sinks/stdout_sinks.cpp index 9bb6ab55..e8720733 100644 --- a/src/sinks/stdout_sinks.cpp +++ b/src/sinks/stdout_sinks.cpp @@ -1,11 +1,12 @@ // Copyright(c) 2015-present, Gabi Melman & spdlog contributors. // Distributed under the MIT License (http://opensource.org/licenses/MIT) -#include "spdlog/sinks/stdout_sinks.h" #include #include "spdlog/pattern_formatter.h" +#include "spdlog/sinks/stdout_sinks.h" +#include "spdlog/details/os.h" // clang-format off #ifdef _WIN32 @@ -23,7 +24,6 @@ // clang-format on namespace spdlog { - namespace sinks { template @@ -61,7 +61,7 @@ void stdout_sink_base::sink_it_(const details::log_msg &msg) { #else memory_buf_t formatted; base_sink::formatter_->format(msg, formatted); - ::fwrite(formatted.data(), sizeof(char), formatted.size(), file_); + details::os::fwrite_bytes(formatted.data(), formatted.size(), file_); #endif // _WIN32 ::fflush(file_); // flush every line to terminal } diff --git a/tests/test_misc.cpp b/tests/test_misc.cpp index 363f9509..8743475b 100644 --- a/tests/test_misc.cpp +++ b/tests/test_misc.cpp @@ -1,7 +1,11 @@ +#include +#include #include "includes.h" #include "spdlog/sinks/ostream_sink.h" +#include "spdlog/details/os.h" #include "test_sink.h" + template std::string log_info(const T &what, spdlog::level logger_level = spdlog::level::info) { std::ostringstream oss; @@ -175,3 +179,32 @@ TEST_CASE("utf8 to utf16 conversion using windows api", "[windows utf]") { REQUIRE(std::wstring(buffer.data(), buffer.size()) == std::wstring(L"\x306d\x3053")); } #endif + +struct auto_closer { + FILE* fp = nullptr; + explicit auto_closer(FILE* f) : fp(f) {} + auto_closer(const auto_closer&) = delete; + auto_closer& operator=(const auto_closer&) = delete; + ~auto_closer() { + fp != nullptr && (std::fclose(fp) != 0); + } +}; + + +TEST_CASE("os::fwrite_bytes", "[os]") { + using spdlog::details::os::fwrite_bytes; + const char* filename = "test_fwrite_bytes.txt"; + const char *msg = "hello"; + { + auto_closer closer(std::fopen(filename, "wb")); + REQUIRE(closer.fp != nullptr); + REQUIRE(fwrite_bytes(msg, std::strlen(msg), closer.fp) == true); + REQUIRE(fwrite_bytes(msg, 0, closer.fp) == true); + std::fflush(closer.fp); + REQUIRE(spdlog::details::os::filesize(closer.fp) == 5); + } + // fwrite_bytes should return false on write failure + auto_closer closer(std::fopen(filename, "r")); + REQUIRE(closer.fp != nullptr); + REQUIRE_FALSE(fwrite_bytes("Hello", 5, closer.fp)); +}