From a5c61688050aff376b24f58a600f9acbfbf627aa Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Sun, 2 Jan 2022 11:51:59 -0600 Subject: [PATCH] refactor: make tr_buildPath() private (#2374) --- libtransmission/platform.cc | 68 +++++++++++++++++++++++------ libtransmission/platform.h | 3 +- libtransmission/session.cc | 29 ++++++------ libtransmission/session.h | 6 +-- libtransmission/torrent.h | 2 +- libtransmission/tr-dht.cc | 6 +-- libtransmission/utils.cc | 45 ------------------- libtransmission/utils.h | 4 -- libtransmission/web.cc | 2 +- tests/libtransmission/utils-test.cc | 9 ---- utils/create.cc | 45 ++++++++++--------- 11 files changed, 103 insertions(+), 116 deletions(-) diff --git a/libtransmission/platform.cc b/libtransmission/platform.cc index 2670861ce..5bafcca6c 100644 --- a/libtransmission/platform.cc +++ b/libtransmission/platform.cc @@ -7,7 +7,9 @@ */ #include +#include #include +#include #include #include #include @@ -48,6 +50,50 @@ using namespace std::literals; +static char* tr_buildPath(char const* first_element, ...) +{ + // pass 1: allocate enough space for the string + va_list vl; + va_start(vl, first_element); + auto bufLen = size_t{}; + for (char const* element = first_element; element != nullptr;) + { + bufLen += strlen(element) + 1; + element = va_arg(vl, char const*); + } + va_end(vl); + char* const buf = tr_new(char, bufLen); + if (buf == nullptr) + { + return nullptr; + } + + /* pass 2: build the string piece by piece */ + char* pch = buf; + va_start(vl, first_element); + for (char const* element = first_element; element != nullptr;) + { + size_t const elementLen = strlen(element); + pch = std::copy_n(element, elementLen, pch); + *pch++ = TR_PATH_DELIMITER; + element = va_arg(vl, char const*); + } + va_end(vl); + + // if nonempty, eat the unwanted trailing slash + if (pch != buf) + { + --pch; + } + + // zero-terminate the string + *pch++ = '\0'; + + /* sanity checks & return */ + TR_ASSERT(pch - buf == (ptrdiff_t)bufLen); + return buf; +} + /*** **** THREADS ***/ @@ -219,32 +265,28 @@ static char const* getHomeDir(void) #define TORRENT_SUBDIR "torrents" #endif -void tr_setConfigDir(tr_session* session, char const* configDir) +void tr_setConfigDir(tr_session* session, std::string_view config_dir) { - session->configDir = tr_strdup(configDir); - - char* path = tr_buildPath(configDir, RESUME_SUBDIR, nullptr); - tr_sys_dir_create(path, TR_SYS_DIR_CREATE_PARENTS, 0777, nullptr); - session->resumeDir = path; - - path = tr_buildPath(configDir, TORRENT_SUBDIR, nullptr); - tr_sys_dir_create(path, TR_SYS_DIR_CREATE_PARENTS, 0777, nullptr); - session->torrentDir = path; + session->config_dir = config_dir; + session->resume_dir = tr_strvPath(config_dir, RESUME_SUBDIR); + session->torrent_dir = tr_strvPath(config_dir, TORRENT_SUBDIR); + tr_sys_dir_create(session->resume_dir.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0777, nullptr); + tr_sys_dir_create(session->torrent_dir.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0777, nullptr); } char const* tr_sessionGetConfigDir(tr_session const* session) { - return session->configDir; + return session->config_dir.c_str(); } char const* tr_getTorrentDir(tr_session const* session) { - return session->torrentDir; + return session->torrent_dir.c_str(); } char const* tr_getResumeDir(tr_session const* session) { - return session->resumeDir; + return session->resume_dir.c_str(); } char const* tr_getDefaultConfigDir(char const* appname) diff --git a/libtransmission/platform.h b/libtransmission/platform.h index 58dfb8f0d..83df0d5a1 100644 --- a/libtransmission/platform.h +++ b/libtransmission/platform.h @@ -13,6 +13,7 @@ #endif #include +#include /** * @addtogroup tr_session Session @@ -25,7 +26,7 @@ * @see tr_getTorrentDir() * @see tr_getWebClientDir() */ -void tr_setConfigDir(tr_session* session, char const* configDir); +void tr_setConfigDir(tr_session* session, std::string_view config_dir); /** @brief return the directory where .resume files are stored */ char const* tr_getResumeDir(tr_session const*); diff --git a/libtransmission/session.cc b/libtransmission/session.cc index 670529d42..2d4bb71d1 100644 --- a/libtransmission/session.cc +++ b/libtransmission/session.cc @@ -472,7 +472,7 @@ void tr_sessionGetSettings(tr_session const* s, tr_variant* d) tr_variantDictAddBool(d, TR_KEY_anti_brute_force_enabled, tr_sessionGetAntiBruteForceEnabled(s)); } -bool tr_sessionLoadSettings(tr_variant* dict, char const* configDir, char const* appName) +bool tr_sessionLoadSettings(tr_variant* dict, char const* config_dir, char const* appName) { TR_ASSERT(tr_variantIsDict(dict)); @@ -485,14 +485,14 @@ bool tr_sessionLoadSettings(tr_variant* dict, char const* configDir, char const* tr_variantFree(&oldDict); /* if caller didn't specify a config dir, use the default */ - if (tr_str_is_empty(configDir)) + if (tr_str_is_empty(config_dir)) { - configDir = tr_getDefaultConfigDir(appName); + config_dir = tr_getDefaultConfigDir(appName); } /* file settings override the defaults */ auto fileSettings = tr_variant{}; - auto const filename = tr_strvPath(configDir, "settings.json"sv); + auto const filename = tr_strvPath(config_dir, "settings.json"sv); auto success = bool{}; if (tr_error* error = nullptr; tr_variantFromFile(&fileSettings, TR_VARIANT_PARSE_JSON, filename.c_str(), &error)) { @@ -510,12 +510,12 @@ bool tr_sessionLoadSettings(tr_variant* dict, char const* configDir, char const* return success; } -void tr_sessionSaveSettings(tr_session* session, char const* configDir, tr_variant const* clientSettings) +void tr_sessionSaveSettings(tr_session* session, char const* config_dir, tr_variant const* clientSettings) { TR_ASSERT(tr_variantIsDict(clientSettings)); tr_variant settings; - auto const filename = tr_strvPath(configDir, "settings.json"sv); + auto const filename = tr_strvPath(config_dir, "settings.json"sv); tr_variantInitDict(&settings, 0); @@ -588,11 +588,11 @@ struct init_data bool done; bool messageQueuingEnabled; tr_session* session; - char const* configDir; + char const* config_dir; tr_variant* clientSettings; }; -tr_session* tr_sessionInit(char const* configDir, bool messageQueuingEnabled, tr_variant* clientSettings) +tr_session* tr_sessionInit(char const* config_dir, bool messageQueuingEnabled, tr_variant* clientSettings) { TR_ASSERT(tr_variantIsDict(clientSettings)); @@ -625,7 +625,7 @@ tr_session* tr_sessionInit(char const* configDir, bool messageQueuingEnabled, tr auto data = init_data{}; data.done = false; data.session = session; - data.configDir = configDir; + data.config_dir = config_dir; data.messageQueuingEnabled = messageQueuingEnabled; data.clientSettings = clientSettings; tr_runInEventThread(session, tr_sessionInitImpl, &data); @@ -724,7 +724,7 @@ static void tr_sessionInitImpl(void* vdata) tr_logSetQueueEnabled(data->messageQueuingEnabled); - tr_setConfigDir(session, data->configDir); + tr_setConfigDir(session, data->config_dir); session->peerMgr = tr_peerMgrNew(session); @@ -735,7 +735,7 @@ static void tr_sessionInitImpl(void* vdata) **/ { - auto const filename = tr_strvPath(session->configDir, "blocklists"sv); + auto const filename = tr_strvPath(session->config_dir, "blocklists"sv); tr_sys_dir_create(filename.c_str(), TR_SYS_DIR_CREATE_PARENTS, 0777, nullptr); loadBlocklists(session); } @@ -2003,9 +2003,6 @@ void tr_sessionClose(tr_session* session) delete session->turtle.minutes; tr_session_id_free(session->session_id); - tr_free(session->configDir); - tr_free(session->resumeDir); - tr_free(session->torrentDir); delete session; } @@ -2290,7 +2287,7 @@ static void loadBlocklists(tr_session* session) auto const isEnabled = session->useBlocklist(); /* walk the blocklist directory... */ - auto const dirname = tr_strvPath(session->configDir, "blocklists"sv); + auto const dirname = tr_strvPath(session->config_dir, "blocklists"sv); auto const odir = tr_sys_dir_open(dirname.c_str(), nullptr); if (odir == TR_BAD_SYS_DIR) @@ -2444,7 +2441,7 @@ int tr_blocklistSetContent(tr_session* session, char const* contentFilename) if (it == std::end(src)) { - auto path = tr_strvJoin(session->configDir, "blocklists"sv, name); + auto path = tr_strvJoin(session->config_dir, "blocklists"sv, name); b = tr_blocklistFileNew(path.c_str(), session->useBlocklist()); src.push_back(b); } diff --git a/libtransmission/session.h b/libtransmission/session.h index 0f30558c8..878187f01 100644 --- a/libtransmission/session.h +++ b/libtransmission/session.h @@ -321,9 +321,9 @@ public: std::map torrentsById; std::map torrentsByHash; - char* configDir; - char* resumeDir; - char* torrentDir; + std::string config_dir; + std::string resume_dir; + std::string torrent_dir; std::list blocklists; struct tr_peerMgr* peerMgr; diff --git a/libtransmission/torrent.h b/libtransmission/torrent.h index 51701f7e3..7f5543cab 100644 --- a/libtransmission/torrent.h +++ b/libtransmission/torrent.h @@ -751,7 +751,7 @@ void tr_torrentGotBlock(tr_torrent* tor, tr_block_index_t blockIndex); /** * @brief Like tr_torrentFindFile(), but splits the filename into base and subpath. * - * If the file is found, "tr_buildPath(base, subpath, nullptr)" + * If the file is found, "tr_strvPath(base, subpath, nullptr)" * will generate the complete filename. * * @return true if the file is found, false otherwise. diff --git a/libtransmission/tr-dht.cc b/libtransmission/tr-dht.cc index 22de6d966..b86add23a 100644 --- a/libtransmission/tr-dht.cc +++ b/libtransmission/tr-dht.cc @@ -218,7 +218,7 @@ static void dht_bootstrap(void* closure) if (!bootstrap_done(cl->session, 0)) { - auto const bootstrap_file = tr_strvPath(cl->session->configDir, "dht.bootstrap"); + auto const bootstrap_file = tr_strvPath(cl->session->config_dir, "dht.bootstrap"); tr_sys_file_t const f = tr_sys_file_open(bootstrap_file.c_str(), TR_SYS_FILE_READ, 0, nullptr); @@ -314,7 +314,7 @@ int tr_dhtInit(tr_session* ss) dht_debug = stderr; } - auto const dat_file = tr_strvPath(ss->configDir, "dht.dat"sv); + auto const dat_file = tr_strvPath(ss->config_dir, "dht.dat"sv); auto benc = tr_variant{}; auto const ok = tr_variantFromFile(&benc, TR_VARIANT_PARSE_BENC, dat_file); @@ -465,7 +465,7 @@ void tr_dhtUninit(tr_session* ss) tr_variantDictAddRaw(&benc, TR_KEY_nodes6, compact6, out6 - compact6); } - auto const dat_file = tr_strvPath(ss->configDir, "dht.dat"); + auto const dat_file = tr_strvPath(ss->config_dir, "dht.dat"); tr_variantToFile(&benc, TR_VARIANT_FMT_BENC, dat_file); tr_variantFree(&benc); } diff --git a/libtransmission/utils.cc b/libtransmission/utils.cc index 5777cf112..310a9ea99 100644 --- a/libtransmission/utils.cc +++ b/libtransmission/utils.cc @@ -401,51 +401,6 @@ bool tr_saveFile(std::string_view filename_in, std::string_view contents, tr_err return true; } -char* tr_buildPath(char const* first_element, ...) -{ - - /* pass 1: allocate enough space for the string */ - va_list vl; - va_start(vl, first_element); - auto bufLen = size_t{}; - for (char const* element = first_element; element != nullptr;) - { - bufLen += strlen(element) + 1; - element = va_arg(vl, char const*); - } - va_end(vl); - char* const buf = tr_new(char, bufLen); - if (buf == nullptr) - { - return nullptr; - } - - /* pass 2: build the string piece by piece */ - char* pch = buf; - va_start(vl, first_element); - for (char const* element = first_element; element != nullptr;) - { - size_t const elementLen = strlen(element); - pch = std::copy_n(element, elementLen, pch); - *pch++ = TR_PATH_DELIMITER; - element = va_arg(vl, char const*); - } - va_end(vl); - - // if nonempty, eat the unwanted trailing slash - if (pch != buf) - { - --pch; - } - - // zero-terminate the string - *pch++ = '\0'; - - /* sanity checks & return */ - TR_ASSERT(pch - buf == (ptrdiff_t)bufLen); - return buf; -} - tr_disk_space tr_dirSpace(std::string_view dir) { if (std::empty(dir)) diff --git a/libtransmission/utils.h b/libtransmission/utils.h index 03219cdc7..56590fb80 100644 --- a/libtransmission/utils.h +++ b/libtransmission/utils.h @@ -82,10 +82,6 @@ bool tr_loadFile(std::vector& setme, std::string_view filename, tr_error** bool tr_saveFile(std::string_view filename, std::string_view contents, tr_error** error = nullptr); -/** @brief build a filename from a series of elements using the - platform's correct directory separator. */ -char* tr_buildPath(char const* first_element, ...) TR_GNUC_NULL_TERMINATED TR_GNUC_MALLOC; - template && ...), bool> = true> std::string& tr_buildBuf(std::string& setme, T... args) { diff --git a/libtransmission/web.cc b/libtransmission/web.cc index 3d6fc4f60..31b56472b 100644 --- a/libtransmission/web.cc +++ b/libtransmission/web.cc @@ -424,7 +424,7 @@ static void tr_webThreadFunc(void* vsession) tr_logAddNamedInfo("web", "NB: invalid certs will show up as 'Could not connect to tracker' like many other errors"); } - auto const str = tr_strvPath(session->configDir, "cookies.txt"); + auto const str = tr_strvPath(session->config_dir, "cookies.txt"); if (tr_sys_path_exists(str.c_str(), nullptr)) { web->cookie_filename = tr_strvDup(str); diff --git a/tests/libtransmission/utils-test.cc b/tests/libtransmission/utils-test.cc index 180e97cfa..1ad7ea4f4 100644 --- a/tests/libtransmission/utils-test.cc +++ b/tests/libtransmission/utils-test.cc @@ -146,15 +146,6 @@ TEST_F(UtilsTest, trStrvDup) tr_free(str); } -TEST_F(UtilsTest, trBuildpath) -{ - auto out = makeString(tr_buildPath("foo", "bar", nullptr)); - EXPECT_EQ("foo" TR_PATH_DELIMITER_STR "bar", out); - - out = makeString(tr_buildPath("", "foo", "bar", nullptr)); - EXPECT_EQ(TR_PATH_DELIMITER_STR "foo" TR_PATH_DELIMITER_STR "bar", out); -} - TEST_F(UtilsTest, trStrvPath) { EXPECT_EQ("foo" TR_PATH_DELIMITER_STR "bar", tr_strvPath("foo", "bar")); diff --git a/utils/create.cc b/utils/create.cc index abaa8cca9..7fe033f1b 100644 --- a/utils/create.cc +++ b/utils/create.cc @@ -7,11 +7,13 @@ */ #include -#include /* fprintf() */ -#include /* strtoul(), EXIT_FAILURE */ -#include /* PRIu32 */ +#include +#include +#include +#include #include + #include #include #include @@ -23,12 +25,15 @@ using namespace std::literals; -static char constexpr MyName[] = "transmission-create"; -static char constexpr Usage[] = "Usage: transmission-create [options] "; +namespace +{ -static uint32_t constexpr KiB = 1024; +char constexpr MyName[] = "transmission-create"; +char constexpr Usage[] = "Usage: transmission-create [options] "; -static auto constexpr Options = std::array{ +uint32_t constexpr KiB = 1024; + +auto constexpr Options = std::array{ { { 'p', "private", "Allow this torrent to only be used with the specified tracker(s)", "p", false, nullptr }, { 'r', "source", "Set the source for private trackers", "r", true, "" }, { 'o', "outfile", "Save the generated .torrent to this filename", "o", true, "" }, @@ -42,16 +47,16 @@ static auto constexpr Options = std::array{ struct app_options { std::vector trackers; + std::string outfile; + char const* comment = nullptr; + char const* infile = nullptr; + char const* source = nullptr; + uint32_t piecesize_kib = 0; bool is_private = false; bool show_version = false; - char const* comment = nullptr; - char const* outfile = nullptr; - char const* infile = nullptr; - uint32_t piecesize_kib = 0; - char const* source = nullptr; }; -static int parseCommandLine(app_options& options, int argc, char const* const* argv) +int parseCommandLine(app_options& options, int argc, char const* const* argv) { int c; char const* optarg; @@ -110,7 +115,7 @@ static int parseCommandLine(app_options& options, int argc, char const* const* a return 0; } -static char* tr_getcwd(void) +char* tr_getcwd(void) { char* result; tr_error* error = nullptr; @@ -127,9 +132,10 @@ static char* tr_getcwd(void) return result; } +} // namespace + int tr_main(int argc, char* argv[]) { - char* out2 = nullptr; tr_metainfo_builder* b = nullptr; tr_logSetLevel(TR_LOG_ERROR); @@ -157,7 +163,7 @@ int tr_main(int argc, char* argv[]) return EXIT_FAILURE; } - if (options.outfile == nullptr) + if (std::empty(options.outfile)) { tr_error* error = nullptr; char* base = tr_sys_path_basename(options.infile, &error); @@ -170,7 +176,7 @@ int tr_main(int argc, char* argv[]) auto const end = tr_strvJoin(base, ".torrent"sv); char* cwd = tr_getcwd(); - options.outfile = out2 = tr_buildPath(cwd, end.c_str(), nullptr); + options.outfile = tr_strvDup(tr_strvPath(cwd, end.c_str())); tr_free(cwd); tr_free(base); } @@ -188,7 +194,7 @@ int tr_main(int argc, char* argv[]) } } - printf("Creating torrent \"%s\"\n", options.outfile); + printf("Creating torrent \"%s\"\n", options.outfile.c_str()); b = tr_metaInfoBuilderCreate(options.infile); @@ -214,7 +220,7 @@ int tr_main(int argc, char* argv[]) tr_makeMetaInfo( b, - options.outfile, + options.outfile.c_str(), std::data(options.trackers), std::size(options.trackers), options.comment, @@ -264,6 +270,5 @@ int tr_main(int argc, char* argv[]) putc('\n', stdout); tr_metaInfoBuilderFree(b); - tr_free(out2); return EXIT_SUCCESS; }