Skip to content

Commit d202054

Browse files
committed
Merge #21052: refactor: Replace fs::unique_path with GetUniquePath(path) calls
1bca2aa Introduce GetUniquePath(base) helper method to replace boost::filesystem::unique_path() which is not available in std::filesystem. (Kiminuo) Pull request description: This PR makes it easier in #20744 to remove our dependency on the `boost::filesystem::unique_path()` function which does not have a direct equivalent in C++17. This PR attempts to re-implement `boost::filesystem::unique_path()` as `GetUniquePath(path)` but the implementations are not meant to be the same. Note: * Boost 1.75.0 implementation of `unique_path`: https://github.com/boostorg/filesystem/blob/9cab675b71e98706886a87afe7c19eb9da568961/src/unique_path.cpp#L235 * In the previous implementation, I attempted to add: ```cpp fs::path GetUniquePath(const fs::path& base) { FastRandomContext rnd; fs::path tmpFile = base / HexStr(rnd.randbytes(8)); return tmpFile; } ``` to `fs.cpp` but this leads to a circular dependency: "fs -> random -> logging -> fs". That is why the modified implementation adds a new file. ACKs for top commit: laanwj: Code review ACK 1bca2aa ryanofsky: Code review ACK 1bca2aa. It's a simple change and extra test coverage is nice Tree-SHA512: f324bdf0e254160c616b5033c3ece33d87db23eb0135acee99346ade7b5cf0d30f3ceefe359a25a8e9b53ba8e4419f459c2bdd369e10fc0152ce95031d1f221c
2 parents d48f9e8 + 1bca2aa commit d202054

File tree

6 files changed

+52
-3
lines changed

6 files changed

+52
-3
lines changed

src/Makefile.am

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ BITCOIN_CORE_H = \
233233
util/check.h \
234234
util/error.h \
235235
util/fees.h \
236+
util/getuniquepath.h \
236237
util/golombrice.h \
237238
util/hasher.h \
238239
util/macros.h \
@@ -556,6 +557,7 @@ libbitcoin_util_a_SOURCES = \
556557
util/bytevectorhash.cpp \
557558
util/error.cpp \
558559
util/fees.cpp \
560+
util/getuniquepath.cpp \
559561
util/hasher.cpp \
560562
util/system.cpp \
561563
util/message.cpp \

src/test/fs_tests.cpp

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <fs.h>
66
#include <test/util/setup_common.h>
77
#include <util/system.h>
8+
#include <util/getuniquepath.h>
89

910
#include <boost/test/unit_test.hpp>
1011

@@ -69,6 +70,21 @@ BOOST_AUTO_TEST_CASE(fsbridge_fstream)
6970
BOOST_CHECK_EQUAL(tmpfile1, fsbridge::AbsPathJoin(tmpfile1, ""));
7071
BOOST_CHECK_EQUAL(tmpfile1, fsbridge::AbsPathJoin(tmpfile1, {}));
7172
}
73+
{
74+
fs::path p1 = GetUniquePath(tmpfolder);
75+
fs::path p2 = GetUniquePath(tmpfolder);
76+
fs::path p3 = GetUniquePath(tmpfolder);
77+
78+
// Ensure that the parent path is always the same.
79+
BOOST_CHECK_EQUAL(tmpfolder, p1.parent_path());
80+
BOOST_CHECK_EQUAL(tmpfolder, p2.parent_path());
81+
BOOST_CHECK_EQUAL(tmpfolder, p3.parent_path());
82+
83+
// Ensure that generated paths are actually different.
84+
BOOST_CHECK(p1 != p2);
85+
BOOST_CHECK(p2 != p3);
86+
BOOST_CHECK(p1 != p3);
87+
}
7288
}
7389

74-
BOOST_AUTO_TEST_SUITE_END()
90+
BOOST_AUTO_TEST_SUITE_END()

src/test/util_tests.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <test/util/setup_common.h>
1414
#include <test/util/str.h>
1515
#include <uint256.h>
16+
#include <util/getuniquepath.h>
1617
#include <util/message.h> // For MessageSign(), MessageVerify(), MESSAGE_MAGIC
1718
#include <util/moneystr.h>
1819
#include <util/spanparsing.h>
@@ -1816,7 +1817,7 @@ BOOST_AUTO_TEST_CASE(test_DirIsWritable)
18161817
BOOST_CHECK_EQUAL(DirIsWritable(tmpdirname), true);
18171818

18181819
// Should not be able to write to a non-existent dir.
1819-
tmpdirname = tmpdirname / fs::unique_path();
1820+
tmpdirname = GetUniquePath(tmpdirname);
18201821
BOOST_CHECK_EQUAL(DirIsWritable(tmpdirname), false);
18211822

18221823
fs::create_directory(tmpdirname);

src/util/getuniquepath.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#include <random.h>
2+
#include <fs.h>
3+
#include <util/strencodings.h>
4+
5+
fs::path GetUniquePath(const fs::path& base)
6+
{
7+
FastRandomContext rnd;
8+
fs::path tmpFile = base / HexStr(rnd.randbytes(8));
9+
return tmpFile;
10+
}

src/util/getuniquepath.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Copyright (c) 2021 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#ifndef BITCOIN_UTIL_GETUNIQUEPATH_H
6+
#define BITCOIN_UTIL_GETUNIQUEPATH_H
7+
8+
#include <fs.h>
9+
10+
/**
11+
* Helper function for getting a unique path
12+
*
13+
* @param[in] base Base path
14+
* @returns base joined with a random 8-character long string.
15+
* @post Returned path is unique with high probability.
16+
*/
17+
fs::path GetUniquePath(const fs::path& base);
18+
19+
#endif // BITCOIN_UTIL_GETUNIQUEPATH_H

src/util/system.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <chainparamsbase.h>
1313
#include <sync.h>
1414
#include <util/check.h>
15+
#include <util/getuniquepath.h>
1516
#include <util/strencodings.h>
1617
#include <util/string.h>
1718
#include <util/translation.h>
@@ -124,7 +125,7 @@ void ReleaseDirectoryLocks()
124125

125126
bool DirIsWritable(const fs::path& directory)
126127
{
127-
fs::path tmpFile = directory / fs::unique_path();
128+
fs::path tmpFile = GetUniquePath(directory);
128129

129130
FILE* file = fsbridge::fopen(tmpFile, "a");
130131
if (!file) return false;

0 commit comments

Comments
 (0)