refactor: remove tr_file.priv (#2349)

This commit is contained in:
Charles Kerr 2021-12-26 16:04:20 -06:00 committed by GitHub
parent fd96a9270b
commit 6149870540
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 88 additions and 140 deletions

View File

@ -57,11 +57,9 @@ static std::string getTorrentFilename(tr_session const* session, tr_info const*
****
***/
bool tr_metainfoAppendSanitizedPathComponent(std::string& out, std::string_view in, bool* is_adjusted)
bool tr_metainfoAppendSanitizedPathComponent(std::string& out, std::string_view in)
{
auto const original_out_len = std::size(out);
auto const original_in = in;
*is_adjusted = false;
// remove leading spaces
auto constexpr leading_test = [](auto ch)
@ -109,16 +107,14 @@ bool tr_metainfoAppendSanitizedPathComponent(std::string& out, std::string_view
break;
}
*is_adjusted = original_in != std::string_view{ out.c_str() + original_out_len };
return std::size(out) > original_out_len;
}
static bool getfile(char** setme, bool* is_adjusted, std::string_view root, tr_variant* path, std::string& buf)
static bool getfile(char** setme, std::string_view root, tr_variant* path, std::string& buf)
{
bool success = false;
*setme = nullptr;
*is_adjusted = false;
if (tr_variantIsList(path))
{
@ -135,16 +131,13 @@ static bool getfile(char** setme, bool* is_adjusted, std::string_view root, tr_v
break;
}
auto is_component_adjusted = bool{};
auto const pos = std::size(buf);
if (!tr_metainfoAppendSanitizedPathComponent(buf, raw, &is_component_adjusted))
if (!tr_metainfoAppendSanitizedPathComponent(buf, raw))
{
continue;
}
buf.insert(std::begin(buf) + pos, TR_PATH_DELIMITER);
*is_adjusted |= is_component_adjusted;
}
}
@ -156,7 +149,6 @@ static bool getfile(char** setme, bool* is_adjusted, std::string_view root, tr_v
if (success)
{
*setme = tr_utf8clean(buf);
*is_adjusted |= buf != *setme;
}
return success;
@ -167,9 +159,8 @@ static char const* parseFiles(tr_info* inf, tr_variant* files, tr_variant const*
int64_t len = 0;
inf->totalSize = 0;
bool is_root_adjusted = false;
auto root_name = std::string{};
if (!tr_metainfoAppendSanitizedPathComponent(root_name, inf->name, &is_root_adjusted))
if (!tr_metainfoAppendSanitizedPathComponent(root_name, inf->name))
{
return "path";
}
@ -202,8 +193,7 @@ static char const* parseFiles(tr_info* inf, tr_variant* files, tr_variant const*
break;
}
bool is_file_adjusted = false;
if (!getfile(&inf->files[i].name, &is_file_adjusted, root_name, path, buf))
if (!getfile(&inf->files[i].name, root_name, path, buf))
{
errstr = "path";
break;
@ -216,7 +206,6 @@ static char const* parseFiles(tr_info* inf, tr_variant* files, tr_variant const*
}
inf->files[i].length = len;
inf->files[i].priv.is_renamed = is_root_adjusted || is_file_adjusted;
inf->totalSize += len;
}
}
@ -227,7 +216,6 @@ static char const* parseFiles(tr_info* inf, tr_variant* files, tr_variant const*
inf->files = tr_new0(tr_file, 1);
inf->files[0].name = tr_strvDup(root_name);
inf->files[0].length = len;
inf->files[0].priv.is_renamed = is_root_adjusted;
inf->totalSize += len;
}
else

View File

@ -21,8 +21,9 @@
#include "transmission.h"
#include "tr-macros.h"
#include "bitfield.h"
#include "torrent.h"
#include "tr-macros.h"
struct tr_error;
struct tr_variant;
@ -38,6 +39,7 @@ struct tr_metainfo_parsed
tr_info info = {};
uint64_t info_dict_length = 0;
std::vector<tr_sha1_digest_t> pieces;
tr_bitfield files_renamed = tr_bitfield{ 0 };
tr_metainfo_parsed() = default;
@ -76,4 +78,4 @@ void tr_metainfoMigrateFile(
enum tr_metainfo_basename_format new_format);
/** @brief Private function that's exposed here only for unit tests */
bool tr_metainfoAppendSanitizedPathComponent(std::string& out, std::string_view in, bool* is_adjusted);
bool tr_metainfoAppendSanitizedPathComponent(std::string& out, std::string_view in);

View File

@ -400,23 +400,10 @@ static uint64_t loadName(tr_variant* dict, tr_torrent* tor)
static void saveFilenames(tr_variant* dict, tr_torrent const* tor)
{
auto const n = tor->fileCount();
bool any_renamed = false;
for (tr_file_index_t i = 0; !any_renamed && i < n; ++i)
tr_variant* const list = tr_variantDictAddList(dict, TR_KEY_files, n);
for (tr_file_index_t i = 0; i < n; ++i)
{
any_renamed = tor->file(i).priv.is_renamed;
}
if (any_renamed)
{
tr_variant* list = tr_variantDictAddList(dict, TR_KEY_files, n);
for (tr_file_index_t i = 0; i < n; ++i)
{
auto const& file = tor->file(i);
tr_variantListAddStrView(list, file.priv.is_renamed ? file.name : "");
}
tr_variantListAddStrView(list, tor->file(i).name);
}
}
@ -433,12 +420,11 @@ static uint64_t loadFilenames(tr_variant* dict, tr_torrent* tor)
for (size_t i = 0; i < n_files && i < n_list; ++i)
{
auto sv = std::string_view{};
if (tr_variantGetStrView(tr_variantListChild(list, i), &sv) && !std::empty(sv))
auto& file = tor->file(i);
if (tr_variantGetStrView(tr_variantListChild(list, i), &sv) && !std::empty(sv) && sv != file.name)
{
auto& file = tor->file(i);
tr_free(file.name);
file.name = tr_strvDup(sv);
file.priv.is_renamed = true;
}
}
@ -487,11 +473,12 @@ static void saveProgress(tr_variant* dict, tr_torrent* tor)
tr_variant* const prog = tr_variantDictAddDict(dict, TR_KEY_progress, 4);
// add the mtimes
auto const n = tor->fileCount();
auto const& mtimes = tor->file_mtimes_;
auto const n = std::size(mtimes);
tr_variant* const l = tr_variantDictAddList(prog, TR_KEY_mtimes, n);
for (tr_file_index_t i = 0; i < n; ++i)
for (auto const& mtime : mtimes)
{
tr_variantListAddInt(l, tor->file(i).priv.mtime);
tr_variantListAddInt(l, mtime);
}
// add the 'checked pieces' bitfield

View File

@ -576,9 +576,9 @@ static void torrentInitFromInfoDict(tr_torrent* tor)
}
tor->fpm_.reset(tor->info);
tor->file_mtimes_.resize(tor->fileCount());
tor->file_priorities_.reset(&tor->fpm_);
tor->files_wanted_.reset(&tor->fpm_);
tor->checked_pieces_ = tr_bitfield{ tor->pieceCount() };
}
@ -2507,8 +2507,7 @@ static void tr_torrentFileCompleted(tr_torrent* tor, tr_file_index_t i)
/* now that the file is complete and closed, we can start watching its
* mtime timestamp for changes to know if we need to reverify pieces */
tr_file& file = tor->file(i);
file.priv.mtime = tr_time();
tor->file_mtimes_[i] = tr_time();
/* if the torrent's current filename isn't the same as the one in the
* metadata -- for example, if it had the ".part" suffix appended to
@ -2517,6 +2516,7 @@ static void tr_torrentFileCompleted(tr_torrent* tor, tr_file_index_t i)
char* sub = nullptr;
if (tr_torrentFindFile2(tor, i, &base, &sub, nullptr))
{
tr_file& file = tor->file(i);
if (strcmp(sub, file.name) != 0)
{
auto const oldpath = tr_strvPath(base, sub);
@ -2975,7 +2975,6 @@ static void renameTorrentFileString(tr_torrent* tor, char const* oldpath, char c
{
tr_free(file.name);
file.name = name;
file.priv.is_renamed = true;
}
}

View File

@ -479,13 +479,16 @@ public:
TR_ASSERT(std::size(checked) == this->pieceCount());
checked_pieces_ = checked;
auto const n = this->fileCount();
this->file_mtimes_.resize(n);
auto filename = std::string{};
for (size_t i = 0, n = this->fileCount(); i < n; ++i)
for (size_t i = 0; i < n; ++i)
{
auto const found = this->findFile(filename, i);
auto const mtime = found ? found->last_modified_at : 0;
this->file(i).priv.mtime = mtime;
this->file_mtimes_[i] = mtime;
// if a file has changed, mark its pieces as unchecked
if (mtime == 0 || mtime != mtimes[i])
@ -709,6 +712,8 @@ public:
tr_file_priorities file_priorities_{ &fpm_ };
tr_files_wanted files_wanted_{ &fpm_ };
std::vector<time_t> file_mtimes_;
private:
void setFilesWanted(tr_file_index_t const* files, size_t n_files, bool wanted, bool is_bootstrapping)
{

View File

@ -1499,21 +1499,12 @@ void tr_torrentVerify(tr_torrent* torrent, tr_verify_done_func callback_func_or_
* tr_info
**********************************************************************/
struct tr_file_priv
{
time_t mtime;
bool is_renamed; // true if we're using a different path from the one in the metainfo; ie, if the user has renamed it */
};
/** @brief a part of tr_info that represents a single file of the torrent's content */
struct tr_file
{
// public
char* name; /* Path to the file */
uint64_t length; /* Length of the file, in bytes */
// libtransmission implementation; do not use
tr_file_priv priv;
};
/** @brief information about a torrent that comes from its metainfo file */

View File

@ -41,55 +41,52 @@ TEST_F(MetainfoTest, sanitize)
{
std::string_view input;
std::string_view expected_output;
bool expected_is_adjusted;
};
auto const tests = std::array<LocalTest, 29>{
// skipped
LocalTest{ ""sv, ""sv, false },
{ "."sv, ""sv, true },
{ ".."sv, ""sv, true },
{ "....."sv, ""sv, true },
{ " "sv, ""sv, true },
{ " . "sv, ""sv, true },
{ ". . ."sv, ""sv, true },
LocalTest{ ""sv, ""sv },
{ "."sv, ""sv },
{ ".."sv, ""sv },
{ "....."sv, ""sv },
{ " "sv, ""sv },
{ " . "sv, ""sv },
{ ". . ."sv, ""sv },
// replaced with '_'
{ "/"sv, "_"sv, true },
{ "////"sv, "____"sv, true },
{ "\\\\"sv, "__"sv, true },
{ "/../"sv, "_.._"sv, true },
{ "foo<bar:baz/boo"sv, "foo_bar_baz_boo"sv, true },
{ "t\0e\x01s\tt\ri\nn\fg"sv, "t_e_s_t_i_n_g"sv, true },
{ "/"sv, "_"sv },
{ "////"sv, "____"sv },
{ "\\\\"sv, "__"sv },
{ "/../"sv, "_.._"sv },
{ "foo<bar:baz/boo"sv, "foo_bar_baz_boo"sv },
{ "t\0e\x01s\tt\ri\nn\fg"sv, "t_e_s_t_i_n_g"sv },
// appended with '_'
{ "con"sv, "con_"sv, true },
{ "cOm4"sv, "cOm4_"sv, true },
{ "LPt9.txt"sv, "LPt9_.txt"sv, true },
{ "NUL.tar.gz"sv, "NUL_.tar.gz"sv, true },
{ "con"sv, "con_"sv },
{ "cOm4"sv, "cOm4_"sv },
{ "LPt9.txt"sv, "LPt9_.txt"sv },
{ "NUL.tar.gz"sv, "NUL_.tar.gz"sv },
// trimmed
{ " foo"sv, "foo"sv, true },
{ "foo "sv, "foo"sv, true },
{ " foo "sv, "foo"sv, true },
{ "foo."sv, "foo"sv, true },
{ "foo..."sv, "foo"sv, true },
{ " foo... "sv, "foo"sv, true },
{ " foo"sv, "foo"sv },
{ "foo "sv, "foo"sv },
{ " foo "sv, "foo"sv },
{ "foo."sv, "foo"sv },
{ "foo..."sv, "foo"sv },
{ " foo... "sv, "foo"sv },
// unmodified
{ "foo"sv, "foo"sv, false },
{ ".foo"sv, ".foo"sv, false },
{ "..foo"sv, "..foo"sv, false },
{ "foo.bar.baz"sv, "foo.bar.baz"sv, false },
{ "null"sv, "null"sv, false },
{ "compass"sv, "compass"sv, false },
{ "foo"sv, "foo"sv },
{ ".foo"sv, ".foo"sv },
{ "..foo"sv, "..foo"sv },
{ "foo.bar.baz"sv, "foo.bar.baz"sv },
{ "null"sv, "null"sv },
{ "compass"sv, "compass"sv },
};
auto out = std::string{};
auto is_adjusted = bool{};
for (auto const& test : tests)
{
out.clear();
auto const success = tr_metainfoAppendSanitizedPathComponent(out, test.input, &is_adjusted);
auto const success = tr_metainfoAppendSanitizedPathComponent(out, test.input);
EXPECT_EQ(!std::empty(out), success);
EXPECT_EQ(test.expected_output, out);
EXPECT_EQ(test.expected_is_adjusted, is_adjusted);
}
}

View File

@ -317,10 +317,6 @@ TEST_F(RenameTest, multifileTorrent)
EXPECT_EQ(expected_files[3], tr_torrentFile(tor, 3).name);
EXPECT_TRUE(testFileExistsAndConsistsOfThisString(tor, 1, expected_contents[1]));
EXPECT_TRUE(testFileExistsAndConsistsOfThisString(tor, 2, expected_contents[2]));
EXPECT_FALSE(files[0].priv.is_renamed);
EXPECT_TRUE(files[1].priv.is_renamed);
EXPECT_TRUE(files[2].priv.is_renamed);
EXPECT_FALSE(files[3].priv.is_renamed);
// (while the branch is renamed: confirm that the .resume file remembers the changes)
tr_torrentSaveResume(tor);
@ -343,11 +339,6 @@ TEST_F(RenameTest, multifileTorrent)
EXPECT_TRUE(testFileExistsAndConsistsOfThisString(tor, i, expected_contents[i]));
}
EXPECT_FALSE(files[0].priv.is_renamed);
EXPECT_TRUE(files[1].priv.is_renamed);
EXPECT_TRUE(files[2].priv.is_renamed);
EXPECT_FALSE(files[3].priv.is_renamed);
/***
**** Test it an incomplete torrent...
***/
@ -439,23 +430,14 @@ TEST_F(RenameTest, multifileTorrent)
"MjpwaWVjZSBsZW5ndGhpMzI3NjhlNjpwaWVjZXMyMDp27buFkmy8ICfNX4nsJmt0Ckm2Ljc6cHJp"
"dmF0ZWkwZWVl");
EXPECT_TRUE(tr_isTorrent(tor));
files = tor->info.files;
// rename prefix of top
EXPECT_EQ(EINVAL, torrentRenameAndWait(tor, "Feli", "FelidaeX"));
EXPECT_STREQ("Felidae", tr_torrentName(tor));
EXPECT_FALSE(files[0].priv.is_renamed);
EXPECT_FALSE(files[1].priv.is_renamed);
EXPECT_FALSE(files[2].priv.is_renamed);
EXPECT_FALSE(files[3].priv.is_renamed);
// rename false path
EXPECT_EQ(EINVAL, torrentRenameAndWait(tor, "Felidae/FelinaeX", "Genus Felinae"));
EXPECT_STREQ("Felidae", tr_torrentName(tor));
EXPECT_FALSE(files[0].priv.is_renamed);
EXPECT_FALSE(files[1].priv.is_renamed);
EXPECT_FALSE(files[2].priv.is_renamed);
EXPECT_FALSE(files[3].priv.is_renamed);
/***
****

View File

@ -94,55 +94,52 @@ TEST_F(TorrentMetainfoTest, sanitize)
{
std::string_view input;
std::string_view expected_output;
bool expected_is_adjusted;
};
auto const tests = std::array<LocalTest, 29>{
// skipped
LocalTest{ ""sv, ""sv, false },
{ "."sv, ""sv, true },
{ ".."sv, ""sv, true },
{ "....."sv, ""sv, true },
{ " "sv, ""sv, true },
{ " . "sv, ""sv, true },
{ ". . ."sv, ""sv, true },
LocalTest{ ""sv, ""sv },
{ "."sv, ""sv },
{ ".."sv, ""sv },
{ "....."sv, ""sv },
{ " "sv, ""sv },
{ " . "sv, ""sv },
{ ". . ."sv, ""sv },
// replaced with '_'
{ "/"sv, "_"sv, true },
{ "////"sv, "____"sv, true },
{ "\\\\"sv, "__"sv, true },
{ "/../"sv, "_.._"sv, true },
{ "foo<bar:baz/boo"sv, "foo_bar_baz_boo"sv, true },
{ "t\0e\x01s\tt\ri\nn\fg"sv, "t_e_s_t_i_n_g"sv, true },
{ "/"sv, "_"sv },
{ "////"sv, "____"sv },
{ "\\\\"sv, "__"sv },
{ "/../"sv, "_.._"sv },
{ "foo<bar:baz/boo"sv, "foo_bar_baz_boo"sv },
{ "t\0e\x01s\tt\ri\nn\fg"sv, "t_e_s_t_i_n_g"sv },
// appended with '_'
{ "con"sv, "con_"sv, true },
{ "cOm4"sv, "cOm4_"sv, true },
{ "LPt9.txt"sv, "LPt9_.txt"sv, true },
{ "NUL.tar.gz"sv, "NUL_.tar.gz"sv, true },
{ "con"sv, "con_"sv },
{ "cOm4"sv, "cOm4_"sv },
{ "LPt9.txt"sv, "LPt9_.txt"sv },
{ "NUL.tar.gz"sv, "NUL_.tar.gz"sv },
// trimmed
{ " foo"sv, "foo"sv, true },
{ "foo "sv, "foo"sv, true },
{ " foo "sv, "foo"sv, true },
{ "foo."sv, "foo"sv, true },
{ "foo..."sv, "foo"sv, true },
{ " foo... "sv, "foo"sv, true },
{ " foo"sv, "foo"sv },
{ "foo "sv, "foo"sv },
{ " foo "sv, "foo"sv },
{ "foo."sv, "foo"sv },
{ "foo..."sv, "foo"sv },
{ " foo... "sv, "foo"sv },
// unmodified
{ "foo"sv, "foo"sv, false },
{ ".foo"sv, ".foo"sv, false },
{ "..foo"sv, "..foo"sv, false },
{ "foo.bar.baz"sv, "foo.bar.baz"sv, false },
{ "null"sv, "null"sv, false },
{ "compass"sv, "compass"sv, false },
{ "foo"sv, "foo"sv },
{ ".foo"sv, ".foo"sv },
{ "..foo"sv, "..foo"sv },
{ "foo.bar.baz"sv, "foo.bar.baz"sv },
{ "null"sv, "null"sv },
{ "compass"sv, "compass"sv },
};
auto out = std::string{};
auto is_adjusted = bool{};
for (auto const& test : tests)
{
out.clear();
auto const success = tr_metainfoAppendSanitizedPathComponent(out, test.input, &is_adjusted);
auto const success = tr_metainfoAppendSanitizedPathComponent(out, test.input);
EXPECT_EQ(!std::empty(out), success);
EXPECT_EQ(test.expected_output, out);
EXPECT_EQ(test.expected_is_adjusted, is_adjusted);
}
}