From 66576c4fd532ac18b8b355ea93d25581a2c15654 Mon Sep 17 00:00:00 2001 From: Kiminuo Date: Fri, 15 Jan 2021 07:26:51 +0000 Subject: test: Clear forced -walletdir setting after wallet init_tests Leaving this value set interfered with the CreateWallet test if it happened to execute later in the test ordering. Specifically it would cause CreateWallet test to write data to the current directory instead of temporary test directory. --- src/wallet/test/init_test_fixture.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src') diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index aa7f1c83d8..f035a70a20 100644 --- a/src/wallet/test/init_test_fixture.cpp +++ b/src/wallet/test/init_test_fixture.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include @@ -37,6 +38,9 @@ InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainNam InitWalletDirTestingSetup::~InitWalletDirTestingSetup() { + gArgs.LockSettings([&](util::Settings& settings) { + settings.forced_settings.erase("walletdir"); + }); fs::current_path(m_cwd); } -- cgit v1.2.3 From da9caa1cedd69702aea44cb44b2fd0a2d6d56916 Mon Sep 17 00:00:00 2001 From: Kiminuo Date: Wed, 13 Jan 2021 15:44:40 +0100 Subject: Replace fs::absolute calls with AbsPathJoin calls This adds better test coverage and will make it easier in #20744 to remove our dependency on the two-argument boost::filesystem::absolute() function which does not have a direct equivalent in C++17. --- src/fs.cpp | 6 ++++++ src/fs.h | 11 +++++++++++ src/rpc/blockchain.cpp | 4 ++-- src/test/fs_tests.cpp | 17 +++++++++++++++++ src/util/system.cpp | 4 ++-- src/wallet/load.cpp | 2 +- src/wallet/wallet.cpp | 2 +- src/wallet/wallettool.cpp | 2 +- 8 files changed, 41 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/fs.cpp b/src/fs.cpp index 7eb0f5ca9f..4f20ca4d28 100644 --- a/src/fs.cpp +++ b/src/fs.cpp @@ -31,6 +31,12 @@ FILE *fopen(const fs::path& p, const char *mode) #endif } +fs::path AbsPathJoin(const fs::path& base, const fs::path& path) +{ + assert(base.is_absolute()); + return fs::absolute(path, base); +} + #ifndef WIN32 static std::string GetErrorReason() diff --git a/src/fs.h b/src/fs.h index dfbecc18e6..d77b90be66 100644 --- a/src/fs.h +++ b/src/fs.h @@ -21,6 +21,17 @@ namespace fs = boost::filesystem; namespace fsbridge { FILE *fopen(const fs::path& p, const char *mode); + /** + * Helper function for joining two paths + * + * @param[in] base Base path + * @param[in] path Path to combine with base + * @returns path unchanged if it is an absolute path, otherwise returns base joined with path. Returns base unchanged if path is empty. + * @pre Base path must be absolute + * @post Returned path will always be absolute + */ + fs::path AbsPathJoin(const fs::path& base, const fs::path& path); + class FileLock { public: diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 3f97f6f3d9..7a336c1ad6 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2399,10 +2399,10 @@ static RPCHelpMan dumptxoutset() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - fs::path path = fs::absolute(request.params[0].get_str(), GetDataDir()); + const fs::path path = fsbridge::AbsPathJoin(GetDataDir(), request.params[0].get_str()); // Write to a temporary path and then move into `path` on completion // to avoid confusion due to an interruption. - fs::path temppath = fs::absolute(request.params[0].get_str() + ".incomplete", GetDataDir()); + const fs::path temppath = fsbridge::AbsPathJoin(GetDataDir(), request.params[0].get_str() + ".incomplete"); if (fs::exists(path)) { throw JSONRPCError( diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp index d02c3613ba..ec487aa3ff 100644 --- a/src/test/fs_tests.cpp +++ b/src/test/fs_tests.cpp @@ -52,6 +52,23 @@ BOOST_AUTO_TEST_CASE(fsbridge_fstream) file >> input_buffer; BOOST_CHECK_EQUAL(input_buffer, "bitcoin"); } + { + // Join an absolute path and a relative path. + fs::path p = fsbridge::AbsPathJoin(tmpfolder, "fs_tests_₿_🏃"); + BOOST_CHECK(p.is_absolute()); + BOOST_CHECK_EQUAL(tmpfile1, p); + } + { + // Join two absolute paths. + fs::path p = fsbridge::AbsPathJoin(tmpfile1, tmpfile2); + BOOST_CHECK(p.is_absolute()); + BOOST_CHECK_EQUAL(tmpfile2, p); + } + { + // Ensure joining with empty paths does not add trailing path components. + BOOST_CHECK_EQUAL(tmpfile1, fsbridge::AbsPathJoin(tmpfile1, "")); + BOOST_CHECK_EQUAL(tmpfile1, fsbridge::AbsPathJoin(tmpfile1, {})); + } } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/util/system.cpp b/src/util/system.cpp index 917d37ccc7..d1fb921642 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -398,7 +398,7 @@ bool ArgsManager::GetSettingsPath(fs::path* filepath, bool temp) const } if (filepath) { std::string settings = GetArg("-settings", BITCOIN_SETTINGS_FILENAME); - *filepath = fs::absolute(temp ? settings + ".tmp" : settings, GetDataDir(/* net_specific= */ true)); + *filepath = fsbridge::AbsPathJoin(GetDataDir(/* net_specific= */ true), temp ? settings + ".tmp" : settings); } return true; } @@ -1311,7 +1311,7 @@ fs::path AbsPathForConfigVal(const fs::path& path, bool net_specific) if (path.is_absolute()) { return path; } - return fs::absolute(path, GetDataDir(net_specific)); + return fsbridge::AbsPathJoin(GetDataDir(net_specific), path); } void ScheduleBatchPriority() diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 036fd4956f..30832f983b 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -62,7 +62,7 @@ bool VerifyWallets(interfaces::Chain& chain) std::set wallet_paths; for (const auto& wallet_file : gArgs.GetArgs("-wallet")) { - const fs::path path = fs::absolute(wallet_file, GetWalletDir()); + const fs::path path = fsbridge::AbsPathJoin(GetWalletDir(), wallet_file); if (!wallet_paths.insert(path).second) { chain.initWarning(strprintf(_("Ignoring duplicate -wallet %s."), wallet_file)); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 918946f9e7..723552860a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3780,7 +3780,7 @@ std::unique_ptr MakeWalletDatabase(const std::string& name, cons // 2. Path to an existing directory. // 3. Path to a symlink to a directory. // 4. For backwards compatibility, the name of a data file in -walletdir. - const fs::path& wallet_path = fs::absolute(name, GetWalletDir()); + const fs::path wallet_path = fsbridge::AbsPathJoin(GetWalletDir(), name); fs::file_type path_type = fs::symlink_status(wallet_path).type(); if (!(path_type == fs::file_not_found || path_type == fs::directory_file || (path_type == fs::symlink_file && fs::is_directory(wallet_path)) || diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index a1bb7343f4..bc90491a2c 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -105,7 +105,7 @@ static void WalletShowInfo(CWallet* wallet_instance) bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command, const std::string& name) { - fs::path path = fs::absolute(name, GetWalletDir()); + const fs::path path = fsbridge::AbsPathJoin(GetWalletDir(), name); if (args.IsArgSet("-format") && command != "createfromdump") { tfm::format(std::cerr, "The -format option can only be used with the \"createfromdump\" command.\n"); -- cgit v1.2.3