diff --git a/cli/cli.cc b/cli/cli.cc index ae273a4f8..e4ca2b83f 100644 --- a/cli/cli.cc +++ b/cli/cli.cc @@ -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); diff --git a/daemon/daemon.cc b/daemon/daemon.cc index 9b798407f..8d7b39bd1 100644 --- a/daemon/daemon.cc +++ b/daemon/daemon.cc @@ -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"); diff --git a/gtk/OptionsDialog.cc b/gtk/OptionsDialog.cc index 9ace69b90..892460049 100644 --- a/gtk/OptionsDialog.cc +++ b/gtk/OptionsDialog.cc @@ -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(); diff --git a/libtransmission/file.h b/libtransmission/file.h index fa7f7a955..d15b35421 100644 --- a/libtransmission/file.h +++ b/libtransmission/file.h @@ -10,12 +10,14 @@ #include // time_t #include #include +#include #ifdef _WIN32 #include #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::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::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::value>, + typename = std::enable_if::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::value>, + typename = std::enable_if::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::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::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()`. * diff --git a/libtransmission/makemeta.cc b/libtransmission/makemeta.cc index 37212ea04..16492753e 100644 --- a/libtransmission/makemeta.cc +++ b/libtransmission/makemeta.cc @@ -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})"), diff --git a/libtransmission/open-files.cc b/libtransmission/open-files.cc index 35db9928c..58b78ed0a 100644 --- a/libtransmission/open-files.cc +++ b/libtransmission/open-files.cc @@ -152,15 +152,9 @@ std::optional 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})"), diff --git a/libtransmission/platform.cc b/libtransmission/platform.cc index e7a36727a..0bf1f30a7 100644 --- a/libtransmission/platform.cc +++ b/libtransmission/platform.cc @@ -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 diff --git a/libtransmission/session-id.cc b/libtransmission/session-id.cc index 730e8d6a9..ccea65cc5 100644 --- a/libtransmission/session-id.cc +++ b/libtransmission/session-id.cc @@ -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); } } diff --git a/libtransmission/torrent-metainfo.cc b/libtransmission/torrent-metainfo.cc index b57aeac2a..72542f9d3 100644 --- a/libtransmission/torrent-metainfo.cc +++ b/libtransmission/torrent-metainfo.cc @@ -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( diff --git a/libtransmission/torrent.cc b/libtransmission/torrent.cc index 427cb6881..dbf6721d0 100644 --- a/libtransmission/torrent.cc +++ b/libtransmission/torrent.cc @@ -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); diff --git a/libtransmission/tr-strbuf.h b/libtransmission/tr-strbuf.h index 52eceeba2..038109eef 100644 --- a/libtransmission/tr-strbuf.h +++ b/libtransmission/tr-strbuf.h @@ -10,6 +10,8 @@ #include +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 diff --git a/libtransmission/utils.cc b/libtransmission/utils.cc index fd700beee..a0a5a4a71 100644 --- a/libtransmission/utils.cc +++ b/libtransmission/utils.cc @@ -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& 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; diff --git a/tests/libtransmission/blocklist-test.cc b/tests/libtransmission/blocklist-test.cc index bfb4fab26..a46b76d38 100644 --- a/tests/libtransmission/blocklist-test.cc +++ b/tests/libtransmission/blocklist-test.cc @@ -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)); diff --git a/tests/libtransmission/file-test.cc b/tests/libtransmission/file-test.cc index 3ebeee010..7fd2a3e68 100644 --- a/tests/libtransmission/file-test.cc +++ b/tests/libtransmission/file-test.cc @@ -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) */ diff --git a/tests/libtransmission/remove-test.cc b/tests/libtransmission/remove-test.cc index 81a25dcde..396e114b7 100644 --- a/tests/libtransmission/remove-test.cc +++ b/tests/libtransmission/remove-test.cc @@ -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); } } diff --git a/tests/libtransmission/rename-test.cc b/tests/libtransmission/rename-test.cc index 3f0edcfcd..fe3cc685a 100644 --- a/tests/libtransmission/rename-test.cc +++ b/tests/libtransmission/rename-test.cc @@ -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); diff --git a/tests/libtransmission/test-fixtures.h b/tests/libtransmission/test-fixtures.h index 24da49662..748182b04 100644 --- a/tests/libtransmission/test-fixtures.h +++ b/tests/libtransmission/test-fixtures.h @@ -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); diff --git a/tests/libtransmission/torrent-metainfo-test.cc b/tests/libtransmission/torrent-metainfo-test.cc index db6d2b46d..1495036c0 100644 --- a/tests/libtransmission/torrent-metainfo-test.cc +++ b/tests/libtransmission/torrent-metainfo-test.cc @@ -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); diff --git a/tests/libtransmission/utils-test.cc b/tests/libtransmission/utils-test.cc index 5a4d49c86..53a975dcd 100644 --- a/tests/libtransmission/utils-test.cc +++ b/tests/libtransmission/utils-test.cc @@ -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 diff --git a/tests/libtransmission/watchdir-test.cc b/tests/libtransmission/watchdir-test.cc index df3ed5d57..dcf4d52dc 100644 --- a/tests/libtransmission/watchdir-test.cc +++ b/tests/libtransmission/watchdir-test.cc @@ -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; }