merge bitcoin#14209, #13878, #14192, #17086: logging and filesystem updates

src/Makefile.test.include

@@ -62,6 +62,7 @@ BITCOIN_TESTS =\
   test/dip0020opcodes_tests.cpp \
   test/evo_deterministicmns_tests.cpp \
   test/evo_simplifiedmns_tests.cpp \
+  test/fs_tests.cpp \
   test/getarg_tests.cpp \
   test/governance_validators_tests.cpp \
   test/hash_tests.cpp \

src/fs.cpp

@@ -102,4 +102,122 @@ bool FileLock::TryLock()
 }
 #endif
 
+std::string get_filesystem_error_message(const fs::filesystem_error& e)
+{
+#ifndef WIN32
+    return e.what();
+#else
+    // Convert from Multi Byte to utf-16
+    std::string mb_string(e.what());
+    int size = MultiByteToWideChar(CP_ACP, 0, mb_string.c_str(), mb_string.size(), nullptr, 0);
+
+    std::wstring utf16_string(size, L'\0');
+    MultiByteToWideChar(CP_ACP, 0, mb_string.c_str(), mb_string.size(), &*utf16_string.begin(), size);
+    // Convert from utf-16 to utf-8
+    return std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>, wchar_t>().to_bytes(utf16_string);
+#endif
+}
+
+#ifdef WIN32
+#ifdef __GLIBCXX__
+
+// reference: https://github.com/gcc-mirror/gcc/blob/gcc-7_3_0-release/libstdc%2B%2B-v3/include/std/fstream#L270
+
+static std::string openmodeToStr(std::ios_base::openmode mode)
+{
+    switch (mode & ~std::ios_base::ate) {
+    case std::ios_base::out:
+    case std::ios_base::out | std::ios_base::trunc:
+        return "w";
+    case std::ios_base::out | std::ios_base::app:
+    case std::ios_base::app:
+        return "a";
+    case std::ios_base::in:
+        return "r";
+    case std::ios_base::in | std::ios_base::out:
+        return "r+";
+    case std::ios_base::in | std::ios_base::out | std::ios_base::trunc:
+        return "w+";
+    case std::ios_base::in | std::ios_base::out | std::ios_base::app:
+    case std::ios_base::in | std::ios_base::app:
+        return "a+";
+    case std::ios_base::out | std::ios_base::binary:
+    case std::ios_base::out | std::ios_base::trunc | std::ios_base::binary:
+        return "wb";
+    case std::ios_base::out | std::ios_base::app | std::ios_base::binary:
+    case std::ios_base::app | std::ios_base::binary:
+        return "ab";
+    case std::ios_base::in | std::ios_base::binary:
+        return "rb";
+    case std::ios_base::in | std::ios_base::out | std::ios_base::binary:
+        return "r+b";
+    case std::ios_base::in | std::ios_base::out | std::ios_base::trunc | std::ios_base::binary:
+        return "w+b";
+    case std::ios_base::in | std::ios_base::out | std::ios_base::app | std::ios_base::binary:
+    case std::ios_base::in | std::ios_base::app | std::ios_base::binary:
+        return "a+b";
+    default:
+        return std::string();
+    }
+}
+
+void ifstream::open(const fs::path& p, std::ios_base::openmode mode)
+{
+    close();
+    m_file = fsbridge::fopen(p, openmodeToStr(mode).c_str());
+    if (m_file == nullptr) {
+        return;
+    }
+    m_filebuf = __gnu_cxx::stdio_filebuf<char>(m_file, mode);
+    rdbuf(&m_filebuf);
+    if (mode & std::ios_base::ate) {
+        seekg(0, std::ios_base::end);
+    }
+}
+
+void ifstream::close()
+{
+    if (m_file != nullptr) {
+        m_filebuf.close();
+        fclose(m_file);
+    }
+    m_file = nullptr;
+}
+
+void ofstream::open(const fs::path& p, std::ios_base::openmode mode)
+{
+    close();
+    m_file = fsbridge::fopen(p, openmodeToStr(mode).c_str());
+    if (m_file == nullptr) {
+        return;
+    }
+    m_filebuf = __gnu_cxx::stdio_filebuf<char>(m_file, mode);
+    rdbuf(&m_filebuf);
+    if (mode & std::ios_base::ate) {
+        seekp(0, std::ios_base::end);
+    }
+}
+
+void ofstream::close()
+{
+    if (m_file != nullptr) {
+        m_filebuf.close();
+        fclose(m_file);
+    }
+    m_file = nullptr;
+}
+#else // __GLIBCXX__
+
+static_assert(sizeof(*fs::path().BOOST_FILESYSTEM_C_STR) == sizeof(wchar_t),
+    "Warning: This build is using boost::filesystem ofstream and ifstream "
+    "implementations which will fail to open paths containing multibyte "
+    "characters. You should delete this static_assert to ignore this warning, "
+    "or switch to a different C++ standard library like the Microsoft C++ "
+    "Standard Library (where boost uses non-standard extensions to construct "
+    "stream objects with wide filenames), or the GNU libstdc++ library (where "
+    "a more complicated workaround has been implemented above).");
+
+#endif // __GLIBCXX__
+#endif // WIN32
+
 } // fsbridge

src/fs.h

@@ -7,6 +7,9 @@
 
 #include <stdio.h>
 #include <string>
+#if defined WIN32 && defined __GLIBCXX__
+#include <ext/stdio_filebuf.h>
+#endif
 
 #include <boost/filesystem.hpp>
 #include <boost/filesystem/fstream.hpp>
@@ -39,6 +42,56 @@ namespace fsbridge {
         void* hFile = (void*)-1; // INVALID_HANDLE_VALUE
 #endif
     };
+
+    std::string get_filesystem_error_message(const fs::filesystem_error& e);
+
+    // GNU libstdc++ specific workaround for opening UTF-8 paths on Windows.
+    //
+    // On Windows, it is only possible to reliably access multibyte file paths through
+    // `wchar_t` APIs, not `char` APIs. But because the C++ standard doesn't
+    // require ifstream/ofstream `wchar_t` constructors, and the GNU library doesn't
+    // provide them (in contrast to the Microsoft C++ library, see
+    // https://stackoverflow.com/questions/821873/how-to-open-an-stdfstream-ofstream-or-ifstream-with-a-unicode-filename/822032#822032),
+    // Boost is forced to fall back to `char` constructors which may not work properly.
+    //
+    // Work around this issue by creating stream objects with `_wfopen` in
+    // combination with `__gnu_cxx::stdio_filebuf`. This workaround can be removed
+    // with an upgrade to C++17, where streams can be constructed directly from
+    // `std::filesystem::path` objects.
+
+#if defined WIN32 && defined __GLIBCXX__
+    class ifstream : public std::istream
+    {
+    public:
+        ifstream() = default;
+        explicit ifstream(const fs::path& p, std::ios_base::openmode mode = std::ios_base::in) { open(p, mode); }
+        ~ifstream() { close(); }
+        void open(const fs::path& p, std::ios_base::openmode mode = std::ios_base::in);
+        bool is_open() { return m_filebuf.is_open(); }
+        void close();
+
+    private:
+        __gnu_cxx::stdio_filebuf<char> m_filebuf;
+        FILE* m_file = nullptr;
+    };
+    class ofstream : public std::ostream
+    {
+    public:
+        ofstream() = default;
+        explicit ofstream(const fs::path& p, std::ios_base::openmode mode = std::ios_base::out) { open(p, mode); }
+        ~ofstream() { close(); }
+        void open(const fs::path& p, std::ios_base::openmode mode = std::ios_base::out);
+        bool is_open() { return m_filebuf.is_open(); }
+        void close();
+
+    private:
+        __gnu_cxx::stdio_filebuf<char> m_filebuf;
+        FILE* m_file = nullptr;
+    };
+#else  // !(WIN32 && __GLIBCXX__)
+    typedef fs::ifstream ifstream;
+    typedef fs::ofstream ofstream;
+#endif // WIN32 && __GLIBCXX__
 };
 
 #endif // BITCOIN_FS_H

src/logging.h

@@ -175,42 +175,31 @@ std::string SafeStringFormat(const std::string& fmt, const Args&... args)
     }
 }
 
-/** Get format string from VA_ARGS for error reporting */
-template<typename... Args> std::string FormatStringFromLogArgs(const char *fmt, const Args&... args) { return fmt; }
-
-static inline void MarkUsed() {}
-template<typename T, typename... Args> static inline void MarkUsed(const T& t, const Args&... args)
-{
-    (void)t;
-    MarkUsed(args...);
-}
-
 // Be conservative when using LogPrintf/error or other things which
 // unconditionally log to debug.log! It should not be the case that an inbound
 // peer can fill up a user's disk with debug.log entries.
 
-#ifdef USE_COVERAGE
-#define LogPrintf(...) do { MarkUsed(__VA_ARGS__); } while(0)
-#define LogPrint(category, ...) do { MarkUsed(__VA_ARGS__); } while(0)
-#else
-#define LogPrintf(...) do { \
-    if (g_logger->Enabled()) { \
-        std::string _log_msg_; /* Unlikely name to avoid shadowing variables */ \
-        try { \
-            _log_msg_ = tfm::format(__VA_ARGS__); \
-        } catch (tinyformat::format_error &e) { \
-            /* Original format string will have newline so don't add one here */ \
-            _log_msg_ = "Error \"" + std::string(e.what()) + "\" while formatting log message: " + FormatStringFromLogArgs(__VA_ARGS__); \
-        } \
-        g_logger->LogPrintStr(_log_msg_); \
-    } \
-} while(0)
-
-#define LogPrint(category, ...) do { \
-    if (LogAcceptCategory((category))) { \
-        LogPrintf(__VA_ARGS__); \
-    } \
-} while(0)
-#endif // USE_COVERAGE
+template <typename... Args>
+static inline void LogPrintf(const char* fmt, const Args&... args)
+{
+    if (g_logger->Enabled()) {
+        std::string log_msg;
+        try {
+            log_msg = tfm::format(fmt, args...);
+        } catch (tinyformat::format_error& fmterr) {
+            /* Original format string will have newline so don't add one here */
+            log_msg = "Error \"" + std::string(fmterr.what()) + "\" while formatting log message: " + fmt;
+        }
+        g_logger->LogPrintStr(log_msg);
+    }
+}
+
+template <typename... Args>
+static inline void LogPrint(const BCLog::LogFlags& category, const Args&... args)
+{
+    if (LogAcceptCategory((category))) {
+        LogPrintf(args...);
+    }
+}
 
 #endif // BITCOIN_LOGGING_H

src/qt/guiutil.cpp

@@ -875,7 +875,7 @@ fs::path static GetAutostartFilePath()
 
 bool GetStartOnSystemStartup()
 {
-    fs::ifstream optionFile(GetAutostartFilePath());
+    fsbridge::ifstream optionFile(GetAutostartFilePath());
     if (!optionFile.good())
         return false;
     // Scan through file for "Hidden=true":
@@ -906,7 +906,7 @@ bool SetStartOnSystemStartup(bool fAutoStart)
 
         fs::create_directories(GetAutostartDir());
 
-        fs::ofstream optionFile(GetAutostartFilePath(), std::ios_base::out|std::ios_base::trunc);
+        fsbridge::ofstream optionFile(GetAutostartFilePath(), std::ios_base::out | std::ios_base::trunc);
         if (!optionFile.good())
             return false;
         std::string chain = gArgs.GetChainName();

src/rpc/protocol.cpp

@@ -13,8 +13,6 @@
 #include <utiltime.h>
 #include <version.h>
 
-#include <fstream>
-
 /**
  * JSON-RPC protocol.  Bitcoin speaks version 1.0 for maximum compatibility,
  * but uses JSON-RPC 1.1/2.0 standards for parts of the 1.0 standard that were
@@ -86,9 +84,9 @@ bool GenerateAuthCookie(std::string *cookie_out)
     /** the umask determines what permissions are used to create this file -
      * these are set to 077 in init.cpp unless overridden with -sysperms.
      */
-    std::ofstream file;
+    fsbridge::ofstream file;
     fs::path filepath_tmp = GetAuthCookieFile(true);
-    file.open(filepath_tmp.string().c_str());
+    file.open(filepath_tmp);
     if (!file.is_open()) {
         LogPrintf("Unable to open cookie authentication file %s for writing\n", filepath_tmp.string());
         return false;
@@ -110,10 +108,10 @@ bool GenerateAuthCookie(std::string *cookie_out)
 
 bool GetAuthCookie(std::string *cookie_out)
 {
-    std::ifstream file;
+    fsbridge::ifstream file;
     std::string cookie;
     fs::path filepath = GetAuthCookieFile();
-    file.open(filepath.string().c_str());
+    file.open(filepath);
     if (!file.is_open())
         return false;
     std::getline(file, cookie);
@@ -129,7 +127,7 @@ void DeleteAuthCookie()
     try {
         fs::remove(GetAuthCookieFile());
     } catch (const fs::filesystem_error& e) {
-        LogPrintf("%s: Unable to remove random auth cookie file: %s\n", __func__, e.what());
+        LogPrintf("%s: Unable to remove random auth cookie file: %s\n", __func__, fsbridge::get_filesystem_error_message(e));
     }
 }
 

src/test/fs_tests.cpp

@@ -0,0 +1,56 @@
+// Copyright (c) 2011-2018 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+//
+#include <fs.h>
+#include <test/test_dash.h>
+
+#include <boost/test/unit_test.hpp>
+
+BOOST_FIXTURE_TEST_SUITE(fs_tests, BasicTestingSetup)
+
+BOOST_AUTO_TEST_CASE(fsbridge_fstream)
+{
+    fs::path tmpfolder = SetDataDir("fsbridge_fstream");
+    // tmpfile1 should be the same as tmpfile2
+    fs::path tmpfile1 = tmpfolder / "fs_tests_₿_🏃";
+    fs::path tmpfile2 = tmpfolder / "fs_tests_₿_🏃";
+    {
+        fsbridge::ofstream file(tmpfile1);
+        file << "bitcoin";
+    }
+    {
+        fsbridge::ifstream file(tmpfile2);
+        std::string input_buffer;
+        file >> input_buffer;
+        BOOST_CHECK_EQUAL(input_buffer, "bitcoin");
+    }
+    {
+        fsbridge::ifstream file(tmpfile1, std::ios_base::in | std::ios_base::ate);
+        std::string input_buffer;
+        file >> input_buffer;
+        BOOST_CHECK_EQUAL(input_buffer, "");
+    }
+    {
+        fsbridge::ofstream file(tmpfile2, std::ios_base::out | std::ios_base::app);
+        file << "tests";
+    }
+    {
+        fsbridge::ifstream file(tmpfile1);
+        std::string input_buffer;
+        file >> input_buffer;
+        BOOST_CHECK_EQUAL(input_buffer, "bitcointests");
+    }
+    {
+        fsbridge::ofstream file(tmpfile2, std::ios_base::out | std::ios_base::trunc);
+        file << "bitcoin";
+    }
+    {
+        fsbridge::ifstream file(tmpfile1);
+        std::string input_buffer;
+        file >> input_buffer;
+        BOOST_CHECK_EQUAL(input_buffer, "bitcoin");
+    }
+}
+
+BOOST_AUTO_TEST_SUITE_END()

src/util.cpp

@@ -813,7 +813,7 @@ void ArgsManager::ReadConfigFiles()
     }
 
     const std::string confPath = GetArg("-conf", BITCOIN_CONF_FILENAME);
-    fs::ifstream stream(GetConfigFile(confPath));
+    fsbridge::ifstream stream(GetConfigFile(confPath));
 
     if (stream.good()) {
         ReadConfigStream(stream);
@@ -829,7 +829,7 @@ void ArgsManager::ReadConfigFiles()
             }
 
             for (const std::string& to_include : includeconf) {
-                fs::ifstream include_config(GetConfigFile(to_include));
+                fsbridge::ifstream include_config(GetConfigFile(to_include));
                 if (include_config.good()) {
                     ReadConfigStream(include_config);
                     LogPrintf("Included configuration file %s\n", to_include.c_str());