Skip to content

Commit e877773

Browse files
authored
Replace directory labels with prefixes (#171)
The `string_view` argument to the `directory` constructor no longer serves as a parent name to the temporary directory; instead, it is now used as a prefix to the directory name This is done because: - This is closer to the `mktemp` documentation examples, which create `tmp.XXXXXX` paths rather than `tmp/XXXXXX` - If one user program created a directory with a parent `label`, then another user program would sometimes fail to create a directory with the same label Also, the parent directory for temporary directories is no longer forced to be created. The C++ standard requires `std::filesystem::temp_directory_path()` to return an existing directory; if it is deleted after getting the path and before creating our directory, we choose to fail as soon as possible
1 parent 9dca484 commit e877773

File tree

8 files changed

+73
-83
lines changed

8 files changed

+73
-83
lines changed

include/tmp/directory

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,19 @@ namespace tmp {
3838
class TMP_EXPORT directory : public entry {
3939
public:
4040
/// Creates a unique temporary directory
41-
/// @param label A label to attach to the temporary directory path
41+
/// @param prefix A prefix to attach to the temporary directory path
4242
/// @throws std::filesystem::filesystem_error if cannot create a directory
43-
/// @throws std::invalid_argument if the label is ill-formatted
44-
explicit directory(std::string_view label = "");
43+
/// @throws std::invalid_argument if the prefix is ill-formatted
44+
explicit directory(std::string_view prefix = "");
4545

4646
/// Creates a unique temporary copy recursively from the given path
47-
/// @param path A path to make a temporary copy from
48-
/// @param label A label to attach to the temporary directory path
47+
/// @param path A path to make a temporary copy from
48+
/// @param prefix A prefix to attach to the temporary directory path
4949
/// @returns The new temporary directory
5050
/// @throws std::filesystem::filesystem_error if given path is not a directory
51-
/// @throws std::invalid_argument if the label is ill-formatted
51+
/// @throws std::invalid_argument if the prefix is ill-formatted
5252
static directory copy(const std::filesystem::path& path,
53-
std::string_view label = "");
53+
std::string_view prefix = "");
5454

5555
/// Concatenates this directory path with a given source
5656
/// @param source A string which represents a path name

src/create.cpp

Lines changed: 37 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -21,32 +21,33 @@
2121
namespace tmp {
2222
namespace {
2323

24-
/// Checks that the given label is valid to attach to a temporary entry path
25-
/// @param[in] label The label to check validity for
26-
/// @returns `true` if the label is valid, `false` otherwise
27-
bool is_label_valid(const fs::path& label) {
28-
return label.empty() || (++label.begin() == label.end() &&
29-
label.is_relative() && !label.has_root_path() &&
30-
label.filename() != "." && label.filename() != "..");
24+
/// Checks if the given prefix is valid to attach to a temporary directory name
25+
/// @param[in] prefix The prefix to check validity for
26+
/// @returns `true` if the prefix is valid, `false` otherwise
27+
bool is_prefix_valid(const fs::path& prefix) {
28+
return prefix.empty() ||
29+
(++prefix.begin() == prefix.end() && prefix.is_relative() &&
30+
!prefix.has_root_path() && prefix.filename() != "." &&
31+
prefix.filename() != "..");
3132
}
3233

33-
/// Checks that the given label is valid to attach to a temporary entry path
34-
/// @param label The label to check validity for
35-
/// @throws std::invalid_argument if the label cannot be attached to a path
36-
void validate_label(const fs::path& label) {
37-
if (!is_label_valid(label)) {
34+
/// Checks that the given prefix is valid to attach to a temporary entry path
35+
/// @param prefix The prefix to check validity for
36+
/// @throws std::invalid_argument if the prefix cannot be attached to a path
37+
void validate_prefix(const fs::path& prefix) {
38+
if (!is_prefix_valid(prefix)) {
3839
throw std::invalid_argument(
39-
"Cannot create a temporary entry: label must be empty or a valid "
40+
"Cannot create a temporary entry: prefix must be empty or a valid "
4041
"single-segmented relative pathname");
4142
}
4243
}
4344

4445
#ifdef _WIN32
45-
/// Creates a temporary path with the given label
46-
/// @note label must be valid
47-
/// @param[in] label A label to attach to the path pattern
46+
/// Creates a temporary path with the given prefix
47+
/// @note prefix must be valid
48+
/// @param[in] prefix A prefix to attach to the path pattern
4849
/// @returns A unique temporary path
49-
fs::path make_path(std::string_view label) {
50+
fs::path make_path(std::string_view prefix) {
5051
constexpr static std::size_t CHARS_IN_GUID = 39;
5152
GUID guid;
5253
CoCreateGuid(&guid);
@@ -58,25 +59,23 @@ fs::path make_path(std::string_view label) {
5859
guid.Data4[3], guid.Data4[4], guid.Data4[5], guid.Data4[6],
5960
guid.Data4[7]);
6061

61-
return fs::temp_directory_path() / label / name;
62+
fs::path path = fs::temp_directory_path() / prefix;
63+
path += name;
64+
65+
return path;
6266
}
6367
#else
64-
/// Creates a temporary path pattern with the given label
65-
/// @note label must be valid
66-
/// @param[in] label A label to attach to the path pattern
68+
/// Creates a temporary path pattern with the given prefix
69+
/// @note prefix must be valid
70+
/// @param[in] prefix A prefix to attach to the path pattern
6771
/// @returns A path pattern for the unique temporary path
68-
fs::path make_pattern(std::string_view label) {
69-
return fs::temp_directory_path() / label / "XXXXXX";
70-
}
71-
#endif
72+
fs::path make_pattern(std::string_view prefix) {
73+
fs::path path = fs::temp_directory_path() / prefix;
74+
path += "XXXXXX"; // TODO: add '.', like `com.github.bugdea1er.tmp.yotR2k`?
7275

73-
/// Creates the parent directory of the given path if it does not exist
74-
/// @param[in] path The path for which the parent directory needs to be created
75-
/// @param[out] ec Parameter for error reporting
76-
/// @returns `true` if a parent directory was newly created, `false` otherwise
77-
bool create_parent(const fs::path& path, std::error_code& ec) {
78-
return fs::create_directories(path.parent_path(), ec);
76+
return path;
7977
}
78+
#endif
8079

8180
#ifdef _WIN32
8281
/// Makes a mode string for the `_wfdopen` function
@@ -147,10 +146,6 @@ std::pair<fs::path, filebuf> create_file(std::ios::openmode mode,
147146
#else
148147
fs::path::string_type path = make_pattern("");
149148
#endif
150-
create_parent(path, ec);
151-
if (ec) {
152-
return {};
153-
}
154149

155150
mode |= std::ios::in | std::ios::out;
156151

@@ -188,11 +183,11 @@ std::pair<fs::path, filebuf> create_file(std::ios::openmode mode,
188183
return std::make_pair(path, std::move(filebuf));
189184
}
190185

191-
fs::path create_directory(std::string_view label) {
192-
validate_label(label); // throws std::invalid_argument with a proper text
186+
fs::path create_directory(std::string_view prefix) {
187+
validate_prefix(prefix); // throws std::invalid_argument with a proper text
193188

194189
std::error_code ec;
195-
fs::path directory = create_directory(label, ec);
190+
fs::path directory = create_directory(prefix, ec);
196191

197192
if (ec) {
198193
throw fs::filesystem_error("Cannot create a temporary directory", ec);
@@ -201,21 +196,17 @@ fs::path create_directory(std::string_view label) {
201196
return directory;
202197
}
203198

204-
fs::path create_directory(std::string_view label, std::error_code& ec) {
205-
if (!is_label_valid(label)) {
199+
fs::path create_directory(std::string_view prefix, std::error_code& ec) {
200+
if (!is_prefix_valid(prefix)) {
206201
ec = std::make_error_code(std::errc::invalid_argument);
207202
return fs::path();
208203
}
209204

210205
#ifdef _WIN32
211-
fs::path::string_type path = make_path(label);
206+
fs::path::string_type path = make_path(prefix);
212207
#else
213-
fs::path::string_type path = make_pattern(label);
208+
fs::path::string_type path = make_pattern(prefix);
214209
#endif
215-
create_parent(path, ec);
216-
if (ec) {
217-
return fs::path();
218-
}
219210

220211
#ifdef _WIN32
221212
if (!CreateDirectory(path.c_str(), nullptr)) {

src/create.hpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,20 @@ std::pair<fs::path, filebuf> create_file(std::ios::openmode mode);
2727
std::pair<fs::path, filebuf> create_file(std::ios::openmode mode,
2828
std::error_code& ec);
2929

30-
/// Creates a temporary directory with the given label in the system's
30+
/// Creates a temporary directory with the given prefix in the system's
3131
/// temporary directory
32-
/// @param[in] label A label to attach to the temporary directory path
32+
/// @param[in] prefix A prefix to attach to the temporary directory name
3333
/// @returns A path to the created temporary directory
3434
/// @throws fs::filesystem_error if cannot create a temporary directory
35-
/// @throws std::invalid_argument if the label is ill-formatted
36-
fs::path create_directory(std::string_view label);
35+
/// @throws std::invalid_argument if the prefix is ill-formatted
36+
fs::path create_directory(std::string_view prefix);
3737

38-
/// Creates a temporary directory with the given label in the system's
38+
/// Creates a temporary directory with the given prefix in the system's
3939
/// temporary directory
40-
/// @param[in] label A label to attach to the temporary directory path
40+
/// @param[in] prefix A prefix to attach to the temporary directory name
4141
/// @param[out] ec Parameter for error reporting
4242
/// @returns A path to the created temporary directory
43-
fs::path create_directory(std::string_view label, std::error_code& ec);
43+
fs::path create_directory(std::string_view prefix, std::error_code& ec);
4444
} // namespace tmp
4545

4646
#endif // TMP_CREATE_H

src/directory.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@ constexpr fs::copy_options copy_options =
1616
fs::copy_options::recursive | fs::copy_options::overwrite_existing;
1717
} // namespace
1818

19-
directory::directory(std::string_view label)
20-
: entry(create_directory(label)) {}
19+
directory::directory(std::string_view prefix)
20+
: entry(create_directory(prefix)) {}
2121

22-
directory directory::copy(const fs::path& path, std::string_view label) {
23-
directory tmpdir = directory(label);
22+
directory directory::copy(const fs::path& path, std::string_view prefix) {
23+
directory tmpdir = directory(prefix);
2424

2525
std::error_code ec;
2626
if (fs::is_directory(path)) {

src/move.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,6 @@ void remove(const fs::path& path) noexcept {
5959
try {
6060
std::error_code ec;
6161
fs::remove_all(path, ec);
62-
63-
fs::path parent = path.parent_path();
64-
if (!fs::equivalent(parent, fs::temp_directory_path(), ec)) {
65-
fs::remove(parent, ec);
66-
}
6762
} catch (const std::bad_alloc& ex) {
6863
static_cast<void>(ex);
6964
}

tests/CMakeLists.txt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ find_package(GTest)
55
add_executable(${PROJECT_NAME} directory.cpp file.cpp)
66
target_link_libraries(${PROJECT_NAME} tmp::tmp GTest::gtest_main)
77
target_compile_definitions(${PROJECT_NAME}
8-
PRIVATE LABEL="com.github.bugdea1er.tmp"
9-
BUILD_DIR="${CMAKE_CURRENT_BINARY_DIR}")
8+
PRIVATE BUILD_DIR="${CMAKE_CURRENT_BINARY_DIR}")
109

1110
# On some platforms (e.g. Windows) CMake doesn't write load paths properly
1211
# This solution to put outputs in the same directory is good enough

tests/directory.cpp

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,20 @@ namespace {
1414

1515
namespace fs = std::filesystem;
1616

17-
/// Tests directory creation with label
18-
TEST(directory, create_with_label) {
19-
directory tmpdir = directory(LABEL);
17+
/// Temporary directory prefix for this test suite
18+
constexpr std::string_view prefix = "com.github.bugdea1er.tmp";
19+
20+
/// Tests directory creation with prefix
21+
TEST(directory, create_with_prefix) {
22+
directory tmpdir = directory(prefix);
2023
fs::path parent = tmpdir.path().parent_path();
2124

2225
EXPECT_TRUE(fs::exists(tmpdir));
2326
EXPECT_TRUE(fs::is_directory(tmpdir));
24-
EXPECT_TRUE(fs::equivalent(parent, fs::temp_directory_path() / LABEL));
27+
EXPECT_TRUE(fs::equivalent(parent, fs::temp_directory_path()));
28+
29+
fs::path::string_type filename = tmpdir.path().filename();
30+
EXPECT_EQ(filename.substr(0, prefix.length()), fs::path(prefix));
2531

2632
fs::perms permissions = fs::status(tmpdir).permissions();
2733
#ifdef _WIN32
@@ -33,8 +39,8 @@ TEST(directory, create_with_label) {
3339
#endif
3440
}
3541

36-
/// Tests directory creation without label
37-
TEST(directory, create_without_label) {
42+
/// Tests directory creation without prefix
43+
TEST(directory, create_without_prefix) {
3844
directory tmpdir = directory();
3945
fs::path parent = tmpdir.path().parent_path();
4046

@@ -43,16 +49,16 @@ TEST(directory, create_without_label) {
4349
EXPECT_TRUE(fs::equivalent(parent, fs::temp_directory_path()));
4450
}
4551

46-
/// Tests multiple directories creation with the same label
52+
/// Tests multiple directories creation with the same prefix
4753
TEST(directory, create_multiple) {
48-
directory fst = directory(LABEL);
49-
directory snd = directory(LABEL);
54+
directory fst = directory(prefix);
55+
directory snd = directory(prefix);
5056

5157
EXPECT_FALSE(fs::equivalent(fst, snd));
5258
}
5359

54-
/// Tests error handling with invalid labels
55-
TEST(directory, create_invalid_label) {
60+
/// Tests error handling with invalid prefixes
61+
TEST(directory, create_invalid_prefix) {
5662
EXPECT_THROW(directory("multi/segment"), std::invalid_argument);
5763
EXPECT_THROW(directory("/root"), std::invalid_argument);
5864
EXPECT_THROW(directory(".."), std::invalid_argument);

tests/meson.build

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ unit_test = executable(
66
'file.cpp',
77
dependencies: [gtest_main_dep, tmp_dep],
88
cpp_args: [
9-
'-DLABEL="com.github.bugdea1er.tmp"',
109
'-DBUILD_DIR="' + meson.current_build_dir() + '"',
1110
],
1211
build_by_default: false,

0 commit comments

Comments
 (0)