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

refactor: add pathbuf and std::string-friendly helpers to tr_sys file and path funcs (#3118) 

* refactor: add sv-friendly tr_sys_dir_create() variant

* refactor: add sv-friendly tr_sys_path_get_info() variant

* refactor: add sv-friendly tr_sys_path_exists() variant

* refactor: add sv-friendly tr_sys_path_remove() variant

* refactor: add sv-friendly tr_sys_path_rename() variant

* fixup! refactor: add sv-friendly tr_sys_path_rename() variant
This commit is contained in:
Charles Kerr 2022-05-21 20:17:00 -05:00 committed by GitHub
parent 690cf50e53
commit c0bb2d40f1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
20 changed files with 125 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
cli/cli.cc
View File

@ -242,14 +242,13 @@ int tr_main(int argc, char* argv[])
if (auto sv = std::string_view{}; tr_variantDictFindStrView(&settings, TR_KEY_download_dir, &sv))
{
// tr_sys_path_exists and tr_sys_dir_create need zero-terminated strs
auto const sz_download_dir = std::string{ sv };
if (!tr_sys_path_exists(sz_download_dir.c_str()))
if (!tr_sys_path_exists(sz_download_dir))
{
tr_error* error = nullptr;
if (!tr_sys_dir_create(sz_download_dir.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0700, &error))
if (!tr_sys_dir_create(sz_download_dir, TR_SYS_DIR_CREATE_PARENTS, 0700, &error))
{
fprintf(stderr, "Unable to create download directory \"%s\": %s\n", sz_download_dir.c_str(), error->message);
tr_error_free(error);

4
daemon/daemon.cc
View File

@ -293,7 +293,7 @@ static auto onFileAdded(tr_watchdir_t dir, char const* name, void* vsession)
tr_logAddInfo(fmt::format(_("Removing torrent file '{path}'"), fmt::arg("path", name)));
if (!tr_sys_path_remove(filename.c_str(), &error))
if (!tr_sys_path_remove(filename, &error))
{
tr_logAddError(fmt::format(
_("Couldn't remove '{path}': {error} ({error_code})"),
@ -906,7 +906,7 @@ CLEANUP:
/* cleanup */
if (pidfile_created)
{
tr_sys_path_remove(sz_pid_filename.c_str());
tr_sys_path_remove(sz_pid_filename);
}
sd_notify(0, "STATUS=\n");

4
gtk/OptionsDialog.cc
View File

@ -186,7 +186,7 @@ void OptionsDialog::Impl::sourceChanged(Gtk::FileChooserButton* b)
{
bool new_file = false;
if (!filename.empty() && (filename_.empty() || !tr_sys_path_is_same(filename.c_str(), filename_.c_str())))
if (!filename.empty() && (filename_.empty() || !tr_sys_path_is_same(filename, filename_)))
{
filename_ = filename;
tr_ctorSetMetainfoFromFile(ctor_.get(), filename_.c_str(), nullptr);
@ -216,7 +216,7 @@ void OptionsDialog::Impl::downloadDirChanged(Gtk::FileChooserButton* b)
{
auto const fname = b->get_filename();
if (!fname.empty() && (downloadDir_.empty() || !tr_sys_path_is_same(fname.c_str(), downloadDir_.c_str())))
if (!fname.empty() && (downloadDir_.empty() || !tr_sys_path_is_same(fname, downloadDir_)))
{
downloadDir_ = fname;
updateTorrent();

46
libtransmission/file.h
View File

@ -10,12 +10,14 @@
#include <ctime> // time_t
#include <string>
#include <string_view>
#include <type_traits>
#ifdef _WIN32
#include <windows.h>
#endif
#include "tr-macros.h"
#include "tr-strbuf.h"
struct tr_error;
@ -159,6 +161,12 @@ bool tr_sys_path_copy(char const* src_path, char const* dst_path, struct tr_erro
*/
bool tr_sys_path_get_info(char const* path, int flags, tr_sys_path_info* info, struct tr_error** error = nullptr);
template<typename T, typename = std::enable_if<std::is_member_function_pointer<decltype(&T::c_str)>::value>>
bool tr_sys_path_get_info(T const& path, int flags, tr_sys_path_info* info, struct tr_error** error = nullptr)
{
return tr_sys_path_get_info(path.c_str(), flags, info, error);
}
/**
* @brief Portability wrapper for `access()`.
*
@ -172,6 +180,12 @@ bool tr_sys_path_get_info(char const* path, int flags, tr_sys_path_info* info, s
*/
bool tr_sys_path_exists(char const* path, struct tr_error** error = nullptr);
template<typename T, typename = std::enable_if<std::is_member_function_pointer<decltype(&T::c_str)>::value>>
bool tr_sys_path_exists(T const& path, struct tr_error** error = nullptr)
{
return tr_sys_path_exists(path.c_str(), error);
}
/**
* @brief Check whether path is relative.
*
@ -198,6 +212,16 @@ bool tr_sys_path_is_relative(std::string_view path);
*/
bool tr_sys_path_is_same(char const* path1, char const* path2, struct tr_error** error = nullptr);
template<
typename T,
typename U,
typename = std::enable_if<std::is_member_function_pointer<decltype(&T::c_str)>::value>,
typename = std::enable_if<std::is_member_function_pointer<decltype(&U::c_str)>::value>>
bool tr_sys_path_is_same(T const& path1, U const& path2, struct tr_error** error = nullptr)
{
return tr_sys_path_is_same(path1.c_str(), path2.c_str(), error);
}
/**
* @brief Portability wrapper for `realpath()`.
*
@ -252,6 +276,16 @@ std::string_view tr_sys_path_dirname(std::string_view path);
*/
bool tr_sys_path_rename(char const* src_path, char const* dst_path, struct tr_error** error = nullptr);
template<
typename T,
typename U,
typename = std::enable_if<std::is_member_function_pointer<decltype(&T::c_str)>::value>,
typename = std::enable_if<std::is_member_function_pointer<decltype(&U::c_str)>::value>>
bool tr_sys_path_rename(T const& src_path, U const& dst_path, struct tr_error** error = nullptr)
{
return tr_sys_path_rename(src_path.c_str(), dst_path.c_str(), error);
}
/**
* @brief Portability wrapper for `remove()`.
*
@ -265,6 +299,12 @@ bool tr_sys_path_rename(char const* src_path, char const* dst_path, struct tr_er
*/
bool tr_sys_path_remove(char const* path, struct tr_error** error = nullptr);
template<typename T, typename = std::enable_if<std::is_member_function_pointer<decltype(&T::c_str)>::value>>
bool tr_sys_path_remove(T const& path, struct tr_error** error = nullptr)
{
return tr_sys_path_remove(path.c_str(), error);
}
/**
* @brief Transform path separators to native ones, in-place.
*
@ -615,6 +655,12 @@ char* tr_sys_dir_get_current(struct tr_error** error = nullptr);
*/
bool tr_sys_dir_create(char const* path, int flags, int permissions, struct tr_error** error = nullptr);
template<typename T, typename = std::enable_if<std::is_member_function_pointer<decltype(&T::c_str)>::value>>
bool tr_sys_dir_create(T const& path, int flags, int permissions, struct tr_error** error = nullptr)
{
return tr_sys_dir_create(path.c_str(), flags, permissions, error);
}
/**
* @brief Portability wrapper for `mkdtemp()`.
*

2
libtransmission/makemeta.cc
View File

@ -55,7 +55,7 @@ static struct FileList* getFiles(std::string_view dir, std::string_view base, st
tr_sys_path_native_separators(std::data(buf));
tr_sys_path_info info;
if (tr_error* error = nullptr; !tr_sys_path_get_info(buf.c_str(), 0, &info, &error))
if (tr_error* error = nullptr; !tr_sys_path_get_info(buf, 0, &info, &error))
{
tr_logAddWarn(fmt::format(
_("Skipping '{path}': {error} ({error_code})"),

12
libtransmission/open-files.cc
View File

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

6
libtransmission/platform.cc
View File

@ -125,8 +125,8 @@ void tr_setConfigDir(tr_session* session, std::string_view config_dir)
session->config_dir = config_dir;
session->resume_dir = tr_strvPath(config_dir, ResumeSubdir);
session->torrent_dir = tr_strvPath(config_dir, TorrentSubdir);
tr_sys_dir_create(session->resume_dir.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0777);
tr_sys_dir_create(session->torrent_dir.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0777);
tr_sys_dir_create(session->resume_dir, TR_SYS_DIR_CREATE_PARENTS, 0777);
tr_sys_dir_create(session->torrent_dir, TR_SYS_DIR_CREATE_PARENTS, 0777);
}
char const* tr_sessionGetConfigDir(tr_session const* session)
@ -428,7 +428,7 @@ std::string tr_getSessionIdDir()
char* program_data_dir = win32_get_known_folder_ex(FOLDERID_ProgramData, KF_FLAG_CREATE);
auto const result = tr_strvPath(program_data_dir, "Transmission");
tr_free(program_data_dir);
tr_sys_dir_create(result.c_str(), 0, 0);
tr_sys_dir_create(result, 0, 0);
return result;
#endif

2
libtransmission/session-id.cc
View File

@ -114,7 +114,7 @@ static void destroy_session_id_lock_file(tr_sys_file_t lock_file, char const* se
if (session_id != nullptr)
{
auto const lock_file_path = get_session_id_lock_file_path(session_id);
tr_sys_path_remove(lock_file_path.c_str());
tr_sys_path_remove(lock_file_path);
}
}

8
libtransmission/torrent-metainfo.cc
View File

@ -521,13 +521,13 @@ bool tr_torrent_metainfo::migrateFile(
std::string_view suffix)
{
auto const old_filename = makeFilename(dirname, name, info_hash_string, BasenameFormat::NameAndPartialHash, suffix);
auto const old_filename_exists = tr_sys_path_exists(old_filename.c_str());
auto const old_filename_exists = tr_sys_path_exists(old_filename);
auto const new_filename = makeFilename(dirname, name, info_hash_string, BasenameFormat::Hash, suffix);
auto const new_filename_exists = tr_sys_path_exists(new_filename.c_str());
auto const new_filename_exists = tr_sys_path_exists(new_filename);
if (old_filename_exists && new_filename_exists)
{
tr_sys_path_remove(old_filename.c_str());
tr_sys_path_remove(old_filename);
return false;
}
@ -536,7 +536,7 @@ bool tr_torrent_metainfo::migrateFile(
return false;
}
if (old_filename_exists && tr_sys_path_rename(old_filename.c_str(), new_filename.c_str()))
if (old_filename_exists && tr_sys_path_rename(old_filename, new_filename))
{
tr_logAddError(
fmt::format(

10
libtransmission/torrent.cc
View File

@ -795,7 +795,7 @@ static void torrentInit(tr_torrent* tor, tr_ctor const* ctor)
// if we don't have a local .torrent or .magnet file already,
// assume the torrent is new
bool const is_new_torrent = !tr_sys_path_exists(filename.c_str());
bool const is_new_torrent = !tr_sys_path_exists(filename);
if (is_new_torrent)
{
@ -2595,14 +2595,14 @@ static int renamePath(tr_torrent* tor, char const* oldpath, char const* newname)
auto const base = tor->isDone() || std::empty(tor->incompleteDir()) ? tor->downloadDir() : tor->incompleteDir();
auto src = tr_strvPath(base, oldpath);
auto src = tr_pathbuf{ base, '/', oldpath };
if (!tr_sys_path_exists(src.c_str())) /* check for it as a partial */
if (!tr_sys_path_exists(src)) /* check for it as a partial */
{
src += tr_torrent_files::PartialFileSuffix;
}
if (tr_sys_path_exists(src.c_str()))
if (tr_sys_path_exists(src))
{
auto const parent = tr_sys_path_dirname(src);
auto const tgt = tr_strvEndsWith(src, tr_torrent_files::PartialFileSuffix) ?
@ -2619,7 +2619,7 @@ static int renamePath(tr_torrent* tor, char const* oldpath, char const* newname)
tmp = errno;
if (!tr_sys_path_rename(src.c_str(), tgt, &error))
if (!tr_sys_path_rename(src, tgt, &error))
{
err = error->code;
tr_error_free(error);

22
libtransmission/tr-strbuf.h
View File

@ -10,6 +10,8 @@
#include <fmt/format.h>
std::string_view tr_sys_path_dirname(std::string_view path);
/**
* A memory buffer which uses a builtin array of N bytes, using heap
* memory only if its string gets too big. Its main use case is building
@ -264,6 +266,26 @@ public:
return c_str();
}
bool popdir() noexcept
{
auto const parent = tr_sys_path_dirname(sv());
auto const changed = parent != sv();
if (changed)
{
if (std::data(parent) == std::data(*this))
{
resize(std::size(parent));
}
else
{
assign(parent);
}
}
return changed;
}
private:
/**
* Ensure that the buffer's string is zero-terminated, e.g. for

11
libtransmission/utils.cc
View File

@ -149,7 +149,7 @@ uint8_t* tr_loadFile(std::string_view path_in, size_t* size, tr_error** error)
/* try to stat the file */
auto info = tr_sys_path_info{};
tr_error* my_error = nullptr;
if (!tr_sys_path_get_info(path.c_str(), 0, &info, &my_error))
if (!tr_sys_path_get_info(path, 0, &info, &my_error))
{
tr_logAddError(fmt::format(
_("Couldn't read '{path}': {error} ({error_code})"),
@ -213,7 +213,7 @@ bool tr_loadFile(std::string_view path_in, std::vector<char>& setme, tr_error**
/* try to stat the file */
auto info = tr_sys_path_info{};
tr_error* my_error = nullptr;
if (!tr_sys_path_get_info(path.c_str(), 0, &info, &my_error))
if (!tr_sys_path_get_info(path, 0, &info, &my_error))
{
tr_logAddError(fmt::format(
_("Couldn't read '{path}': {error} ({error_code})"),
@ -301,7 +301,7 @@ bool tr_saveFile(std::string_view filename_in, std::string_view contents, tr_err
}
// If we saved it to disk successfully, move it from '.tmp' to the correct filename
if (!tr_sys_file_close(fd, error) || !ok || !tr_sys_path_rename(tmp.c_str(), filename.c_str(), error))
if (!tr_sys_file_close(fd, error) || !ok || !tr_sys_path_rename(tmp, filename, error))
{
return false;
}
@ -968,8 +968,9 @@ 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);
std::empty(newdir) || !tr_sys_dir_create(std::string{ newdir }.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0777, error))
auto newdir = tr_pathbuf{ newpath.sv() };
newdir.popdir();
if (!tr_sys_dir_create(newdir, TR_SYS_DIR_CREATE_PARENTS, 0777, error))
{
tr_error_prefix(error, "Unable to create directory for new file: ");
return false;

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

@ -46,7 +46,7 @@ protected:
void createFileWithContents(char const* path, char const* contents)
{
auto const dir = tr_sys_path_dirname(path);
tr_sys_dir_create(dir.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0700);
tr_sys_dir_create(dir, TR_SYS_DIR_CREATE_PARENTS, 0700);
auto const fd = tr_sys_file_open(path, TR_SYS_FILE_WRITE | TR_SYS_FILE_CREATE | TR_SYS_FILE_TRUNCATE, 0600);
blockingFileWrite(fd, contents, strlen(contents));

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

@ -23,6 +23,7 @@
#include "error.h"
#include "file.h"
#include "tr-macros.h"
#include "tr-strbuf.h"
#include "test-fixtures.h"
@ -126,8 +127,7 @@ protected:
}
auto const path_part = std::string{ path, size_t(slash_pos - path + 1) };
if (!tr_sys_path_get_info(path_part.c_str(), TR_SYS_PATH_NO_FOLLOW, &info) ||
if (!tr_sys_path_get_info(path_part, TR_SYS_PATH_NO_FOLLOW, &info) ||
(info.type != TR_SYS_PATH_IS_FILE && info.type != TR_SYS_PATH_IS_DIRECTORY))
{
return false;
@ -456,6 +456,8 @@ TEST_F(FileTest, pathIsRelative)
TEST_F(FileTest, pathIsSame)
{
// NOLINTBEGIN(readability-suspicious-call-argument)
auto const test_dir = createTestDir(currentTestName());
auto const path1 = tr_pathbuf{ test_dir, "/a"sv };
@ -655,6 +657,8 @@ TEST_F(FileTest, pathIsSame)
tr_sys_path_remove(path3);
tr_sys_path_remove(path2);
tr_sys_path_remove(path1);
// NOLINTEND(readability-suspicious-call-argument)
}
TEST_F(FileTest, pathResolve)
@ -862,6 +866,11 @@ TEST_F(FileTest, pathDirname)
{
EXPECT_EQ(expected, tr_sys_path_dirname(input)) << "input[" << input << "] expected [" << expected << "] actual ["
<< tr_sys_path_dirname(input) << ']' << std::endl;
auto path = tr_pathbuf{ input };
path.popdir();
EXPECT_EQ(expected, path) << "input[" << input << "] expected [" << expected << "] actual [" << path << ']'
<< std::endl;
}
/* TODO: is_same(dirname(x) + '/' + basename(x), x) */

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

@ -176,14 +176,13 @@ protected:
for (tr_file_index_t i = 0, n = files.fileCount(); i < n; ++i)
{
auto filename = tr_pathbuf{ parent, '/', files.path(i) };
createFileWithContents(filename, std::data(Content), std::size(Content));
paths.emplace(filename);
auto walk = tr_pathbuf{ parent, '/', files.path(i) };
createFileWithContents(walk, std::data(Content), std::size(Content));
paths.emplace(walk);
auto walk = std::string_view{ filename.sv() };
while (!tr_sys_path_is_same(parent, std::string{ walk }.c_str()))
while (!tr_sys_path_is_same(parent, walk))
{
walk = tr_sys_path_dirname(walk);
walk.popdir();
paths.emplace(walk);
}
}

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(std::string{ tr_sys_path_dirname(str) }.c_str());
tr_sys_path_remove(std::string{ tr_sys_path_dirname(str) });
tr_free(str);
sync();
blockingTorrentVerify(tor);

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

@ -182,9 +182,10 @@ protected:
{
auto const tmperr = errno;
auto const dir = tr_sys_path_dirname(path);
auto dir = tr_pathbuf{ path };
dir.popdir();
tr_error* error = nullptr;
tr_sys_dir_create(std::string{ dir }.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0700, &error);
tr_sys_dir_create(dir, TR_SYS_DIR_CREATE_PARENTS, 0700, &error);
EXPECT_EQ(nullptr, error) << "path[" << path << "] dir[" << dir << "] " << *error;
errno = tmperr;
@ -313,7 +314,7 @@ private:
auto q = TR_KEY_download_dir;
auto const download_dir = tr_variantDictFindStrView(settings, q, &sv) ? tr_strvPath(sandboxDir(), sv) :
tr_strvPath(sandboxDir(), "Downloads");
tr_sys_dir_create(download_dir.data(), TR_SYS_DIR_CREATE_PARENTS, 0700);
tr_sys_dir_create(download_dir, TR_SYS_DIR_CREATE_PARENTS, 0700);
tr_variantDictAddStr(settings, q, download_dir.data());
// incomplete dir
@ -410,8 +411,9 @@ protected:
auto const suffix = std::string_view{ partial ? ".part" : "" };
auto const filename = tr_pathbuf{ base, '/', subpath, suffix };
auto const dirname = tr_sys_path_dirname(filename);
tr_sys_dir_create(std::string{ dirname }.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0700);
auto dirname = tr_pathbuf{ filename.sv() };
dirname.popdir();
tr_sys_dir_create(dirname, 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);

2
tests/libtransmission/torrent-metainfo-test.cc
View File

@ -216,7 +216,7 @@ TEST_F(TorrentMetainfoTest, ctorSaveContents)
EXPECT_EQ(src_contents, tgt_contents);
// cleanup
EXPECT_TRUE(tr_sys_path_remove(tgt_filename.c_str(), &error));
EXPECT_TRUE(tr_sys_path_remove(tgt_filename, &error));
EXPECT_EQ(nullptr, error) << *error;
tr_error_clear(&error);
tr_ctorFree(ctor);

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

@ -379,7 +379,7 @@ TEST_F(UtilsTest, saveFile)
EXPECT_EQ(contents, sv);
// remove the tempfile
EXPECT_TRUE(tr_sys_path_remove(filename.c_str(), &error));
EXPECT_TRUE(tr_sys_path_remove(filename, &error));
EXPECT_EQ(nullptr, error) << *error;
// try saving a file to a path that doesn't exist

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

@ -94,7 +94,7 @@ protected:
path += TR_PATH_DELIMITER;
path += name;
tr_sys_dir_create(path.c_str(), 0, 0700);
tr_sys_dir_create(path, 0, 0700);
return path;
}