mirror
/
transmission
mirror of https://github.com/transmission/transmission
1
0
Fork 0
Code Issues Releases Wiki Activity

perf: tr_sys_path_dirname() returns a std::string_view (#2990) 

* refactor: use nodejs impl of path.win32.dirname()

* refactor: use nodejs impl of path.posix.dirname()
This commit is contained in:
Charles Kerr 2022-05-21 11:10:58 -05:00 committed by GitHub
parent 2e25370cc5
commit 690cf50e53
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 270 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
libtransmission/file-posix.cc
View File

@ -398,17 +398,50 @@ std::string tr_sys_path_basename(std::string_view path, tr_error** error)
return {};
}
std::string tr_sys_path_dirname(std::string_view path, tr_error** error)
// This function is adapted from Node.js's path.posix.dirname() function,
// which is copyrighted by Joyent, Inc. and other Node contributors
// and is distributed under MIT (SPDX:MIT) license.
std::string_view tr_sys_path_dirname(std::string_view path)
{
auto tmp = tr_pathbuf{ path };
auto const len = std::size(path);
if (char const* ret = dirname(std::data(tmp)); ret != nullptr)
if (len == 0U)
{
return ret;
return "."sv;
}
set_system_error(error, errno);
return {};
auto const has_root = path[0] == '/';
auto end = std::string_view::npos;
auto matched_slash = bool{ true };
for (auto i = len - 1; i >= 1U; --i)
{
if (path[i] == '/')
{
if (!matched_slash)
{
end = i;
break;
}
}
else
{
// We saw the first non-path separator
matched_slash = false;
}
}
if (end == std::string_view::npos)
{
return has_root ? "/"sv : "."sv;
}
if (has_root && end == 1)
{
return "//"sv;
}
return path.substr(0, end);
}
bool tr_sys_path_rename(char const* src_path, char const* dst_path, tr_error** error)

146
libtransmission/file-win32.cc
View File

@ -687,62 +687,128 @@ std::string tr_sys_path_basename(std::string_view path, tr_error** error)
return { name, size_t(end - name) };
}
std::string tr_sys_path_dirname(std::string_view path, tr_error** error)
[[nodiscard]] static bool isWindowsDeviceRoot(char ch) noexcept
{
if (std::empty(path))
return isalpha(static_cast<int>(ch)) != 0;
}
[[nodiscard]] static constexpr bool isPathSeparator(char ch) noexcept
{
return ch == '/' || ch == '\\';
}
// This function is adapted from Node.js's path.win32.dirname() function,
// which is copyrighted by Joyent, Inc. and other Node contributors
// and is distributed under MIT (SPDX:MIT) license.
std::string_view tr_sys_path_dirname(std::string_view path)
{
auto const len = std::size(path);
if (len == 0)
{
return ".";
return "."sv;
}
if (!is_valid_path(path))
if (len == 1)
{
set_system_error(error, ERROR_PATH_NOT_FOUND);
return {};
return isPathSeparator(path[0]) ? path : "."sv;
}
bool const is_unc = is_unc_path(path);
auto root_end = std::string_view::npos;
auto offset = std::string_view::size_type{ 0 };
if (is_unc && path[2] == '\0')
// Try to match a root
if (isPathSeparator(path[0]))
{
return std::string{ path };
// Possible UNC root
root_end = offset = 1;
if (isPathSeparator(path[1]))
{
// Matched double path separator at beginning
std::string_view::size_type j = 2;
std::string_view::size_type last = j;
// Match 1 or more non-path separators
while (j < len && !isPathSeparator(path[j]))
{
j++;
}
if (j < len && j != last)
{
// Matched!
last = j;
// Match 1 or more path separators
while (j < len && isPathSeparator(path[j]))
{
j++;
}
if (j < len && j != last)
{
// Matched!
last = j;
// Match 1 or more non-path separators
while (j < len && !isPathSeparator(path[j]))
{
j++;
}
if (j == len)
{
// We matched a UNC root only
return path;
}
if (j != last)
{
// We matched a UNC root with leftovers
// Offset by 1 to include the separator after the UNC root to
// treat it as a "normal root" on top of a (UNC) root
root_end = offset = j + 1;
}
}
}
}
// Possible device root
}
else if (isWindowsDeviceRoot(path[0]) && path[1] == ':')
{
root_end = len > 2 && isPathSeparator(path[2]) ? 3 : 2;
offset = root_end;
}
char const* const begin = std::data(path);
char const* end = begin + std::size(path);
while (end > begin && is_slash(*(end - 1)))
auto end = std::string_view::npos;
auto matched_slash = bool{ true };
for (std::string_view::size_type i = len - 1; i >= offset; --i)
{
--end;
if (isPathSeparator(path[i]))
{
if (!matched_slash)
{
end = i;
break;
}
}
else
{
// We saw the first non-path separator
matched_slash = false;
}
if (i <= offset)
{
break;
}
}
if (end == begin)
if (end == std::string_view::npos)
{
return "/";
if (root_end == std::string_view::npos)
{
return "."sv;
}
end = root_end;
}
char const* name = end;
while (name > begin && *(name - 1) != ':' && !is_slash(*(name - 1)))
{
--name;
}
while (name > begin && is_slash(*(name - 1)))
{
--name;
}
if (name == begin)
{
return is_unc ? "\\\\" : ".";
}
if (name > begin && *(name - 1) == ':' && *name != '\0' && !is_slash(*name))
{
return fmt::format("{}:.", begin[0]);
}
return { begin, size_t(name - begin) };
return path.substr(0, end);
}
bool tr_sys_path_rename(char const* src_path, char const* dst_path, tr_error** error)

4
libtransmission/file.h
View File

@ -230,15 +230,13 @@ std::string tr_sys_path_basename(std::string_view path, struct tr_error** error
* @brief Portability wrapper for `dirname()`.
*
* @param[in] path Path to file or directory.
* @param[out] error Pointer to error object. Optional, pass `nullptr` if you
* are not interested in error details.
*
* @return Pointer to newly allocated buffer containing directory (parent path;
* last path component removed) on success (use @ref tr_free to free it
* when no longer needed), `nullptr` otherwise (with `error` set
* accordingly).
*/
std::string tr_sys_path_dirname(std::string_view path, struct tr_error** error = nullptr);
std::string_view tr_sys_path_dirname(std::string_view path);
/**
* @brief Portability wrapper for `rename()`.

11
libtransmission/open-files.cc
View File

@ -152,20 +152,15 @@ std::optional<tr_sys_file_t> tr_open_files::get(
tr_error* error = nullptr;
if (writable)
{
auto const dir = tr_sys_path_dirname(filename, &error);
auto const dir = tr_sys_path_dirname(filename);
if (std::empty(dir))
{
tr_logAddError(fmt::format(
_("Couldn't create '{path}': {error} ({error_code})"),
fmt::arg("path", filename),
fmt::arg("error", error->message),
fmt::arg("error_code", error->code)));
tr_error_free(error);
tr_logAddError(fmt::format(_("Couldn't create '{path}'"), fmt::arg("path", filename)));
return {};
}
if (!tr_sys_dir_create(dir.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0777, &error))
if (!tr_sys_dir_create(std::string{ dir }.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0777, &error))
{
tr_logAddError(fmt::format(
_("Couldn't create '{path}': {error} ({error_code})"),

4
libtransmission/utils.cc
View File

@ -968,8 +968,8 @@ bool tr_moveFile(std::string_view oldpath_in, std::string_view newpath_in, tr_er
}
// ensure the target directory exists
if (auto const newdir = tr_sys_path_dirname(newpath, error);
std::empty(newdir) || !tr_sys_dir_create(newdir.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0777, error))
if (auto const newdir = tr_sys_path_dirname(newpath);
std::empty(newdir) || !tr_sys_dir_create(std::string{ newdir }.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0777, error))
{
tr_error_prefix(error, "Unable to create directory for new file: ");
return false;

138
tests/libtransmission/file-test.cc
View File

@ -8,6 +8,7 @@
#include <ostream>
#include <string>
#include <string_view>
#include <utility>
#ifndef _WIN32
#include <sys/types.h>
@ -181,6 +182,32 @@ protected:
}
}
static void testPathXname(
XnameTestData const* data,
size_t data_size,
std::string_view (*func)(std::string_view, tr_error**))
{
for (size_t i = 0; i < data_size; ++i)
{
tr_error* err = nullptr;
auto const name = func(data[i].input, &err);
std::cerr << __FILE__ << ':' << __LINE__ << " in [" << data[i].input << "] out [" << name << ']' << std::endl;
if (data[i].output != nullptr)
{
EXPECT_NE(""sv, name);
EXPECT_EQ(nullptr, err) << *err;
EXPECT_EQ(std::string{ data[i].output }, name);
}
else
{
EXPECT_EQ(""sv, name);
EXPECT_NE(nullptr, err);
tr_error_clear(&err);
}
}
}
static void testDirReadImpl(tr_pathbuf const& path, bool* have1, bool* have2)
{
*have1 = *have2 = false;
@ -687,7 +714,7 @@ TEST_F(FileTest, pathResolve)
#endif
}
TEST_F(FileTest, pathBasenameDirname)
TEST_F(FileTest, pathBasename)
{
auto const common_xname_tests = std::vector<XnameTestData>{
XnameTestData{ "/", "/" },
@ -725,7 +752,7 @@ TEST_F(FileTest, pathBasenameDirname)
};
testPathXname(common_xname_tests.data(), common_xname_tests.size(), tr_sys_path_basename);
testPathXname(common_xname_tests.data(), common_xname_tests.size(), tr_sys_path_dirname);
// testPathXname(common_xname_tests.data(), common_xname_tests.size(), tr_sys_path_dirname);
auto const basename_tests = std::vector<XnameTestData>{
XnameTestData{ "a", "a" },
@ -751,36 +778,91 @@ TEST_F(FileTest, pathBasenameDirname)
};
testPathXname(basename_tests.data(), basename_tests.size(), tr_sys_path_basename);
}
auto const dirname_tests = std::vector<XnameTestData>{
XnameTestData{ "/a/b/c", "/a/b" },
{ "a/b/c", "a/b" },
{ "a/b/c/", "a/b" },
{ "a", "." },
{ "a/", "." },
TEST_F(FileTest, pathDirname)
{
#ifdef _WIN32
{ "C:\\a/b\\c", "C:\\a/b" },
{ "C:\\a/b\\c\\", "C:\\a/b" },
{ "C:\\a/b", "C:\\a" },
{ "C:/a", "C:" },
{ "C:", "C:" },
{ "C:/", "C:" },
{ "C:\\", "C:" },
{ "c:a/b", "c:a" },
{ "c:a", "c:." },
{ "c:.", "c:." },
{ "\\\\a\\b\\c", "\\\\a\\b" },
{ "\\\\a\\b\\c/", "\\\\a\\b" },
{ "//a/b", "//a" },
{ "//1.2.3.4/b", "//1.2.3.4" },
{ "\\\\a", "\\\\" },
{ "\\\\1.2.3.4", "\\\\" },
{ "\\\\", "\\\\" },
{ "a/b\\c", "a/b" },
static auto constexpr DirnameTests = std::array<std::pair<std::string_view, std::string_view>, 48>{ {
{ "C:\\a/b\\c"sv, "C:\\a/b"sv },
{ "C:\\a/b\\c\\"sv, "C:\\a/b"sv },
{ "C:\\a/b"sv, "C:\\a"sv },
{ "C:/a"sv, "C:/"sv },
{ "C:"sv, "C:"sv },
{ "C:/"sv, "C:/"sv },
{ "c:a/b"sv, "c:a"sv },
{ "c:a"sv, "c:"sv },
{ "\\\\a"sv, "\\"sv },
{ "\\\\1.2.3.4"sv, "\\"sv },
{ "\\\\"sv, "\\"sv },
{ "a/b\\c"sv, "a/b"sv },
// taken from Node.js unit tests
// https://github.com/nodejs/node/blob/e46c680bf2b211bbd52cf959ca17ee98c7f657f5/test/parallel/test-path-dirname.js
{ "c:\\"sv, "c:\\"sv },
{ "c:\\foo"sv, "c:\\"sv },
{ "c:\\foo\\"sv, "c:\\"sv },
{ "c:\\foo\\bar"sv, "c:\\foo"sv },
{ "c:\\foo\\bar\\"sv, "c:\\foo"sv },
{ "c:\\foo\\bar\\baz"sv, "c:\\foo\\bar"sv },
{ "c:\\foo bar\\baz"sv, "c:\\foo bar"sv },
{ "\\"sv, "\\"sv },
{ "\\foo"sv, "\\"sv },
{ "\\foo\\"sv, "\\"sv },
{ "\\foo\\bar"sv, "\\foo"sv },
{ "\\foo\\bar\\"sv, "\\foo"sv },
{ "\\foo\\bar\\baz"sv, "\\foo\\bar"sv },
{ "\\foo bar\\baz"sv, "\\foo bar"sv },
{ "c:"sv, "c:"sv },
{ "c:foo"sv, "c:"sv },
{ "c:foo\\"sv, "c:"sv },
{ "c:foo\\bar"sv, "c:foo"sv },
{ "c:foo\\bar\\"sv, "c:foo"sv },
{ "c:foo\\bar\\baz"sv, "c:foo\\bar"sv },
{ "c:foo bar\\baz"sv, "c:foo bar"sv },
{ "file:stream"sv, "."sv },
{ "dir\\file:stream"sv, "dir"sv },
{ "\\\\unc\\share"sv, "\\\\unc\\share"sv },
{ "\\\\unc\\share\\foo"sv, "\\\\unc\\share\\"sv },
{ "\\\\unc\\share\\foo\\"sv, "\\\\unc\\share\\"sv },
{ "\\\\unc\\share\\foo\\bar"sv, "\\\\unc\\share\\foo"sv },
{ "\\\\unc\\share\\foo\\bar\\"sv, "\\\\unc\\share\\foo"sv },
{ "\\\\unc\\share\\foo\\bar\\baz"sv, "\\\\unc\\share\\foo\\bar"sv },
{ "/a/b/"sv, "/a"sv },
{ "/a/b"sv, "/a"sv },
{ "/a"sv, "/"sv },
{ ""sv, "."sv },
{ "/"sv, "/"sv },
{ "////"sv, "/"sv },
{ "foo"sv, "."sv },
} };
#else
static auto constexpr DirnameTests = std::array<std::pair<std::string_view, std::string_view>, 15>{ {
// taken from Node.js unit tests
// https://github.com/nodejs/node/blob/e46c680bf2b211bbd52cf959ca17ee98c7f657f5/test/parallel/test-path-dirname.js
{ "/a/b/"sv, "/a"sv },
{ "/a/b"sv, "/a"sv },
{ "/a"sv, "/"sv },
{ ""sv, "."sv },
{ "/"sv, "/"sv },
{ "////"sv, "/"sv },
{ "//a"sv, "//"sv },
{ "foo"sv, "."sv },
// taken from dirname(3) manpage
{ "usr"sv, "."sv },
{ "/usr/lib", "/usr"sv },
{ "/usr/"sv, "/"sv },
{ "/usr/"sv, "/"sv },
{ "/"sv, "/"sv },
{ "."sv, "."sv },
{ ".."sv, "."sv },
} };
#endif
};
testPathXname(dirname_tests.data(), dirname_tests.size(), tr_sys_path_dirname);
for (auto const& [input, expected] : DirnameTests)
{
EXPECT_EQ(expected, tr_sys_path_dirname(input)) << "input[" << input << "] expected [" << expected << "] actual ["
<< tr_sys_path_dirname(input) << ']' << std::endl;
}
/* TODO: is_same(dirname(x) + '/' + basename(x), x) */
}

9
tests/libtransmission/remove-test.cc
View File

@ -180,10 +180,11 @@ protected:
createFileWithContents(filename, std::data(Content), std::size(Content));
paths.emplace(filename);
while (!tr_sys_path_is_same(parent, filename))
auto walk = std::string_view{ filename.sv() };
while (!tr_sys_path_is_same(parent, std::string{ walk }.c_str()))
{
filename = tr_sys_path_dirname(filename);
paths.emplace(filename);
walk = tr_sys_path_dirname(walk);
paths.emplace(walk);
}
}
@ -301,7 +302,7 @@ TEST_F(RemoveTest, LeavesNonJunkAlone)
EXPECT_EQ(expected_tree, getSubtreeContents(parent));
files.remove(parent, "tmpdir_prefix"sv, sysPathRemove);
expected_tree = { parent, tr_sys_path_dirname(nonjunk_file), nonjunk_file.c_str() };
expected_tree = { parent, std::string{ tr_sys_path_dirname(nonjunk_file) }, nonjunk_file.c_str() };
EXPECT_EQ(expected_tree, getSubtreeContents(parent));
}

2
tests/libtransmission/rename-test.cc
View File

@ -350,7 +350,7 @@ TEST_F(RenameTest, multifileTorrent)
str = tr_torrentFindFile(tor, 2);
EXPECT_NE(nullptr, str);
tr_sys_path_remove(str);
tr_sys_path_remove(tr_sys_path_dirname(str).c_str());
tr_sys_path_remove(std::string{ tr_sys_path_dirname(str) }.c_str());
tr_free(str);
sync();
blockingTorrentVerify(tor);

2
tests/libtransmission/subprocess-test.cc
View File

@ -35,7 +35,7 @@ std::string getTestProgramPath(std::string const& filename)
{
auto const exe_path = makeString(tr_sys_path_resolve(testing::internal::GetArgvs().front().data()));
auto const exe_dir = tr_sys_path_dirname(exe_path);
return exe_dir + TR_PATH_DELIMITER + filename;
return std::string{ exe_dir } + TR_PATH_DELIMITER + filename;
}
class SubprocessTest

4
tests/libtransmission/test-fixtures.h
View File

@ -184,7 +184,7 @@ protected:
auto const dir = tr_sys_path_dirname(path);
tr_error* error = nullptr;
tr_sys_dir_create(dir.data(), TR_SYS_DIR_CREATE_PARENTS, 0700, &error);
tr_sys_dir_create(std::string{ dir }.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0700, &error);
EXPECT_EQ(nullptr, error) << "path[" << path << "] dir[" << dir << "] " << *error;
errno = tmperr;
@ -411,7 +411,7 @@ protected:
auto const filename = tr_pathbuf{ base, '/', subpath, suffix };
auto const dirname = tr_sys_path_dirname(filename);
tr_sys_dir_create(dirname.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0700);
tr_sys_dir_create(std::string{ dirname }.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0700);
auto fd = tr_sys_file_open(filename, TR_SYS_FILE_WRITE | TR_SYS_FILE_CREATE | TR_SYS_FILE_TRUNCATE, 0600);
auto const file_size = metainfo->fileSize(i);