Merge bitcoin/bitcoin#29040: refactor: Remove pre-C++20 code, fs::path cleanup

6666713041 refactor: Rename fs::path::u8string() to fs::path::utf8string() (MarcoFalke)
856c88776f ArgsManager: return path by value from GetBlocksDirPath() (Vasil Dimov)
fa3d9304e8 refactor: Remove pre-C++20 fs code (MarcoFalke)
fa00098e1a Add tests for C++20 std::u8string (MarcoFalke)
fa2bac08c2 refactor: Avoid copy/move in fs.h (MarcoFalke)
faea30227b refactor: Use C++20 std::chrono::days (MarcoFalke)

Pull request description:

  This:

  * Removes dead code.
  * Avoids unused copies in some places.
  * Adds copies in other places for safety.

ACKs for top commit:
  achow101:
    ACK 6666713041
  ryanofsky:
    Code review ACK 6666713041. Just documentation change since last review.
  stickies-v:
    re-ACK 6666713041

Tree-SHA512: 6176e44f30b310d51632ec2d3827c3819905d0ddc6a4b57acfcb6cfa1f9735176da75ee8ed4a4abd1296cb0b83bee9374cc6f91ffac87c19b63c435eeadf3f46
pull/29087/head
Ava Chow 11 months ago
commit 1b2dedbf5c
No known key found for this signature in database
GPG Key ID: 17565732E08E5E41

@ -1406,7 +1406,7 @@ A few guidelines for introducing and reviewing new RPC interfaces:
- *Rationale*: User-facing consistency.
- Use `fs::path::u8string()` and `fs::u8path()` functions when converting path
- Use `fs::path::u8string()`/`fs::path::utf8string()` and `fs::u8path()` functions when converting path
to JSON strings, not `fs::PathToString` and `fs::PathFromString`
- *Rationale*: JSON strings are Unicode strings, not byte strings, and

@ -34,7 +34,7 @@ static void WalletCreate(benchmark::Bench& bench, bool encrypted)
fs::path wallet_path = test_setup->m_path_root / strprintf("test_wallet_%d", random.rand32()).c_str();
bench.run([&] {
auto wallet = CreateWallet(context, wallet_path.u8string(), /*load_on_start=*/std::nullopt, options, status, error_string, warnings);
auto wallet = CreateWallet(context, wallet_path.utf8string(), /*load_on_start=*/std::nullopt, options, status, error_string, warnings);
assert(status == DatabaseStatus::SUCCESS);
assert(wallet != nullptr);

@ -277,7 +277,7 @@ fs::path ArgsManager::GetPathArg(std::string arg, const fs::path& default_value)
return result.has_filename() ? result : result.parent_path();
}
const fs::path& ArgsManager::GetBlocksDirPath() const
fs::path ArgsManager::GetBlocksDirPath() const
{
LOCK(cs_args);
fs::path& path = m_cached_blocks_path;
@ -302,7 +302,7 @@ const fs::path& ArgsManager::GetBlocksDirPath() const
return path;
}
const fs::path& ArgsManager::GetDataDir(bool net_specific) const
fs::path ArgsManager::GetDataDir(bool net_specific) const
{
LOCK(cs_args);
fs::path& path = net_specific ? m_cached_network_datadir_path : m_cached_datadir_path;

@ -215,21 +215,21 @@ protected:
*
* @return Blocks path which is network specific
*/
const fs::path& GetBlocksDirPath() const;
fs::path GetBlocksDirPath() const;
/**
* Get data directory path
*
* @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
*/
const fs::path& GetDataDirBase() const { return GetDataDir(false); }
fs::path GetDataDirBase() const { return GetDataDir(false); }
/**
* Get data directory path with appended network identifier
*
* @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
*/
const fs::path& GetDataDirNet() const { return GetDataDir(true); }
fs::path GetDataDirNet() const { return GetDataDir(true); }
/**
* Clear cached directory paths
@ -420,7 +420,7 @@ private:
* @param net_specific Append network identifier to the returned path
* @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
*/
const fs::path& GetDataDir(bool net_specific) const;
fs::path GetDataDir(bool net_specific) const;
/**
* Return -regtest/-signet/-testnet/-chain= setting as a ChainType enum if a

@ -669,7 +669,7 @@ fs::path QStringToPath(const QString &path)
QString PathToQString(const fs::path &path)
{
return QString::fromStdString(path.u8string());
return QString::fromStdString(path.utf8string());
}
QString NetworkToQString(Network net)
@ -723,8 +723,7 @@ QString ConnectionTypeToQString(ConnectionType conn_type, bool prepend_direction
QString formatDurationStr(std::chrono::seconds dur)
{
using days = std::chrono::duration<int, std::ratio<86400>>; // can remove this line after C++20
const auto d{std::chrono::duration_cast<days>(dur)};
const auto d{std::chrono::duration_cast<std::chrono::days>(dur)};
const auto h{std::chrono::duration_cast<std::chrono::hours>(dur - d)};
const auto m{std::chrono::duration_cast<std::chrono::minutes>(dur - d - h)};
const auto s{std::chrono::duration_cast<std::chrono::seconds>(dur - d - h - m)};

@ -2601,7 +2601,7 @@ static RPCHelpMan dumptxoutset()
if (fs::exists(path)) {
throw JSONRPCError(
RPC_INVALID_PARAMETER,
path.u8string() + " already exists. If you are sure this is what you want, "
path.utf8string() + " already exists. If you are sure this is what you want, "
"move it out of the way first");
}
@ -2610,7 +2610,7 @@ static RPCHelpMan dumptxoutset()
if (afile.IsNull()) {
throw JSONRPCError(
RPC_INVALID_PARAMETER,
"Couldn't open file " + temppath.u8string() + " for writing.");
"Couldn't open file " + temppath.utf8string() + " for writing.");
}
NodeContext& node = EnsureAnyNodeContext(request.context);
@ -2618,7 +2618,7 @@ static RPCHelpMan dumptxoutset()
node, node.chainman->ActiveChainstate(), afile, path, temppath);
fs::rename(temppath, path);
result.pushKV("path", path.u8string());
result.pushKV("path", path.utf8string());
return result;
},
};
@ -2690,7 +2690,7 @@ UniValue CreateUTXOSnapshot(
result.pushKV("coins_written", maybe_stats->coins_count);
result.pushKV("base_hash", tip->GetBlockHash().ToString());
result.pushKV("base_height", tip->nHeight);
result.pushKV("path", path.u8string());
result.pushKV("path", path.utf8string());
result.pushKV("txoutset_hash", maybe_stats->hashSerialized.ToString());
result.pushKV("nchaintx", tip->nChainTx);
return result;
@ -2743,7 +2743,7 @@ static RPCHelpMan loadtxoutset()
if (afile.IsNull()) {
throw JSONRPCError(
RPC_INVALID_PARAMETER,
"Couldn't open file " + path.u8string() + " for reading.");
"Couldn't open file " + path.utf8string() + " for reading.");
}
SnapshotMetadata metadata;

@ -806,7 +806,7 @@ static RPCHelpMan savemempool()
}
UniValue ret(UniValue::VOBJ);
ret.pushKV("filename", dump_path.u8string());
ret.pushKV("filename", dump_path.utf8string());
return ret;
},

@ -245,7 +245,7 @@ static RPCHelpMan getrpcinfo()
UniValue result(UniValue::VOBJ);
result.pushKV("active_commands", active_commands);
const std::string path = LogInstance().m_file_path.u8string();
const std::string path = LogInstance().m_file_path.utf8string();
UniValue log_path(UniValue::VSTR, path);
result.pushKV("logpath", log_path);

@ -17,9 +17,12 @@ BOOST_FIXTURE_TEST_SUITE(fs_tests, BasicTestingSetup)
BOOST_AUTO_TEST_CASE(fsbridge_pathtostring)
{
std::string u8_str = "fs_tests_₿_🏃";
std::u8string str8{u8"fs_tests_₿_🏃"};
BOOST_CHECK_EQUAL(fs::PathToString(fs::PathFromString(u8_str)), u8_str);
BOOST_CHECK_EQUAL(fs::u8path(u8_str).u8string(), u8_str);
BOOST_CHECK_EQUAL(fs::PathFromString(u8_str).u8string(), u8_str);
BOOST_CHECK_EQUAL(fs::u8path(u8_str).utf8string(), u8_str);
BOOST_CHECK_EQUAL(fs::path(str8).utf8string(), u8_str);
BOOST_CHECK(fs::path(str8).u8string() == str8);
BOOST_CHECK_EQUAL(fs::PathFromString(u8_str).utf8string(), u8_str);
BOOST_CHECK_EQUAL(fs::PathToString(fs::u8path(u8_str)), u8_str);
#ifndef WIN32
// On non-windows systems, verify that arbitrary byte strings containing
@ -46,7 +49,7 @@ BOOST_AUTO_TEST_CASE(fsbridge_fstream)
fs::path tmpfolder = m_args.GetDataDirBase();
// tmpfile1 should be the same as tmpfile2
fs::path tmpfile1 = tmpfolder / fs::u8path("fs_tests_₿_🏃");
fs::path tmpfile2 = tmpfolder / fs::u8path("fs_tests_₿_🏃");
fs::path tmpfile2 = tmpfolder / fs::path(u8"fs_tests_₿_🏃");
{
std::ofstream file{tmpfile1};
file << "bitcoin";

@ -1,4 +1,4 @@
// Copyright (c) 2017-2022 The Bitcoin Core developers
// Copyright (c) 2017-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
@ -18,6 +18,7 @@
#endif
#include <cassert>
#include <cerrno>
#include <string>
namespace fsbridge {
@ -130,4 +131,4 @@ std::string get_filesystem_error_message(const fs::filesystem_error& e)
#endif
}
} // fsbridge
} // namespace fsbridge

@ -1,4 +1,4 @@
// Copyright (c) 2017-2022 The Bitcoin Core developers
// Copyright (c) 2017-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
@ -14,6 +14,8 @@
#include <ios>
#include <ostream>
#include <string>
#include <system_error>
#include <type_traits>
#include <utility>
/** Filesystem operations and types */
@ -35,7 +37,7 @@ public:
// Allow path objects arguments for compatibility.
path(std::filesystem::path path) : std::filesystem::path::path(std::move(path)) {}
path& operator=(std::filesystem::path path) { std::filesystem::path::operator=(std::move(path)); return *this; }
path& operator/=(std::filesystem::path path) { std::filesystem::path::operator/=(path); return *this; }
path& operator/=(const std::filesystem::path& path) { std::filesystem::path::operator/=(path); return *this; }
// Allow literal string arguments, which are safe as long as the literals are ASCII.
path(const char* c) : std::filesystem::path(c) {}
@ -52,12 +54,15 @@ public:
// Disallow std::string conversion method to avoid locale-dependent encoding on windows.
std::string string() const = delete;
std::string u8string() const
/**
* Return a UTF-8 representation of the path as a std::string, for
* compatibility with code using std::string. For code using the newer
* std::u8string type, it is more efficient to call the inherited
* std::filesystem::path::u8string method instead.
*/
std::string utf8string() const
{
const auto& utf8_str{std::filesystem::path::u8string()};
// utf8_str might either be std::string (C++17) or std::u8string
// (C++20). Convert both to std::string. This method can be removed
// after switching to C++20.
const std::u8string& utf8_str{std::filesystem::path::u8string()};
return std::string{utf8_str.begin(), utf8_str.end()};
}
@ -69,11 +74,7 @@ public:
static inline path u8path(const std::string& utf8_str)
{
#if __cplusplus < 202002L
return std::filesystem::u8path(utf8_str);
#else
return std::filesystem::path(std::u8string{utf8_str.begin(), utf8_str.end()});
#endif
}
// Disallow implicit std::string conversion for absolute to avoid
@ -97,9 +98,9 @@ static inline auto quoted(const std::string& s)
}
// Allow safe path append operations.
static inline path operator/(path p1, path p2)
static inline path operator/(path p1, const path& p2)
{
p1 /= std::move(p2);
p1 /= p2;
return p1;
}
static inline path operator/(path p1, const char* p2)
@ -140,7 +141,7 @@ static inline bool copy_file(const path& from, const path& to, copy_options opti
* Because \ref PathToString and \ref PathFromString functions don't specify an
* encoding, they are meant to be used internally, not externally. They are not
* appropriate to use in applications requiring UTF-8, where
* fs::path::u8string() and fs::u8path() methods should be used instead. Other
* fs::path::u8string() / fs::path::utf8string() and fs::u8path() methods should be used instead. Other
* applications could require still different encodings. For example, JSON, XML,
* or URI applications might prefer to use higher-level escapes (\uXXXX or
* &XXXX; or %XX) instead of multibyte encoding. Rust, Python, Java applications
@ -154,13 +155,13 @@ static inline std::string PathToString(const path& path)
// use here, because these methods encode the path using C++'s narrow
// multibyte encoding, which on Windows corresponds to the current "code
// page", which is unpredictable and typically not able to represent all
// valid paths. So fs::path::u8string() and
// valid paths. So fs::path::utf8string() and
// fs::u8path() functions are used instead on Windows. On
// POSIX, u8string/u8path functions are not safe to use because paths are
// POSIX, u8string/utf8string/u8path functions are not safe to use because paths are
// not always valid UTF-8, so plain string methods which do not transform
// the path there are used.
#ifdef WIN32
return path.u8string();
return path.utf8string();
#else
static_assert(std::is_same<path::string_type, std::string>::value, "PathToString not implemented on this platform");
return path.std::filesystem::path::string();

@ -734,7 +734,7 @@ RPCHelpMan dumpwallet()
* It may also avoid other security issues.
*/
if (fs::exists(filepath)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, filepath.u8string() + " already exists. If you are sure this is what you want, move it out of the way first");
throw JSONRPCError(RPC_INVALID_PARAMETER, filepath.utf8string() + " already exists. If you are sure this is what you want, move it out of the way first");
}
std::ofstream file;
@ -828,7 +828,7 @@ RPCHelpMan dumpwallet()
file.close();
UniValue reply(UniValue::VOBJ);
reply.pushKV("filename", filepath.u8string());
reply.pushKV("filename", filepath.utf8string());
return reply;
},

@ -169,7 +169,7 @@ static RPCHelpMan listwalletdir()
UniValue wallets(UniValue::VARR);
for (const auto& path : ListDatabases(GetWalletDir())) {
UniValue wallet(UniValue::VOBJ);
wallet.pushKV("name", path.u8string());
wallet.pushKV("name", path.utf8string());
wallets.push_back(wallet);
}
@ -806,7 +806,7 @@ static RPCHelpMan migratewallet()
if (res->solvables_wallet) {
r.pushKV("solvables_name", res->solvables_wallet->GetName());
}
r.pushKV("backup_path", res->backup_path.u8string());
r.pushKV("backup_path", res->backup_path.utf8string());
return r;
},

Loading…
Cancel
Save