From 25e53cd7999dfca8e9ad53b0033e89deae1484fd Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Sun, 17 Nov 2013 08:02:16 +0000 Subject: [PATCH] fix set_naem() on file_storage actually affecting save paths --- ChangeLog | 1 + include/libtorrent/file_storage.hpp | 31 +++++++---- src/file_storage.cpp | 82 +++++++++++++++++++---------- test/test_file_storage.cpp | 14 ++++- 4 files changed, 89 insertions(+), 39 deletions(-) diff --git a/ChangeLog b/ChangeLog index 281b3780c..e3af59482 100644 --- a/ChangeLog +++ b/ChangeLog @@ -25,6 +25,7 @@ * fix uTP edge case where udp socket buffer fills up * fix nagle implementation in uTP + * fix set_naem() on file_storage actually affecting save paths * fix large file support issue on mingw * add some error handling to set_piece_hashes() * fix completed-on timestamp to not be clobbered on each startup diff --git a/include/libtorrent/file_storage.hpp b/include/libtorrent/file_storage.hpp index 11b65285a..5a95ba2a6 100644 --- a/include/libtorrent/file_storage.hpp +++ b/include/libtorrent/file_storage.hpp @@ -121,26 +121,28 @@ namespace libtorrent internal_file_entry() : name(NULL) , offset(0) - , symlink_index(not_a_symlink) , size(0) + , symlink_index(not_a_symlink) , name_len(name_is_owned) , pad_file(false) , hidden_attribute(false) , executable_attribute(false) , symlink_attribute(false) + , no_root_dir(false) , path_index(-1) {} internal_file_entry(file_entry const& e) : name(NULL) , offset(e.offset) - , symlink_index(not_a_symlink) , size(e.size) + , symlink_index(not_a_symlink) , name_len(name_is_owned) , pad_file(e.pad_file) , hidden_attribute(e.hidden_attribute) , executable_attribute(e.executable_attribute) , symlink_attribute(e.symlink_attribute) + , no_root_dir(false) , path_index(-1) { set_name(e.path.c_str()); @@ -167,18 +169,18 @@ namespace libtorrent enum { name_is_owned = (1<<12)-1, - not_a_symlink = (1<<16)-1 + not_a_symlink = (1<<15)-1 }; // the offset of this file inside the torrent boost::uint64_t offset:48; - // index into file_storage::m_symlinks or not_a_symlink - // if this is not a symlink - boost::uint64_t symlink_index:16; - // the size of this file boost::uint64_t size:48; + // index into file_storage::m_symlinks or not_a_symlink + // if this is not a symlink + boost::uint64_t symlink_index:15; + // the number of characters in the name. If this is // name_is_owned, name is null terminated and owned by this object // (i.e. it should be freed in the destructor). If @@ -189,14 +191,20 @@ namespace libtorrent boost::uint64_t hidden_attribute:1; boost::uint64_t executable_attribute:1; boost::uint64_t symlink_attribute:1; + + // if this is true, don't include m_name as part of the + // path to this file + boost::uint64_t no_root_dir:1; + // the index into file_storage::m_paths. To get // the full path to this file, concatenate the path // from that array with the 'name' field in // this struct - // if path_index == -2, it means the filename + // values for path_index include: + // -1 means no path (i.e. single file torrent) + // -2, it means the filename // in this field contains the full, absolute path // to the file - // -1 means no path (i.e. single file torrent) int path_index; }; @@ -527,7 +535,10 @@ namespace libtorrent std::vector m_file_base; // all unique paths files have. The internal_file_entry::path_index - // points into this array + // points into this array. The paths don't include the root directory + // name for multi-file torrents. The m_name field need to be + // prepended to these paths, and the filename of a specific file + // entry appended, to form full file paths std::vector m_paths; // name of torrent. For multi-file torrents diff --git a/src/file_storage.cpp b/src/file_storage.cpp index 817df7c22..babd5a2d0 100644 --- a/src/file_storage.cpp +++ b/src/file_storage.cpp @@ -78,29 +78,47 @@ namespace libtorrent return; } std::string parent = parent_path(fname); + if (parent.empty()) { e.path_index = -1; + return; + } + + if (parent.size() >= m_name.size() + && parent.compare(0, m_name.size(), m_name) == 0 + && (parent.size() == m_name.size() +#ifdef TORRENT_WINDOWS + || parent[m_name.size()] == '\\' +#endif + || parent[m_name.size()] == '/' + )) + { + parent.erase(parent.begin(), parent.begin() + m_name.size() + + (m_name.size() == parent.size()?0:1)); + e.no_root_dir = false; } else { - // do we already have this path in the path list? - std::vector::reverse_iterator p - = std::find(m_paths.rbegin(), m_paths.rend(), parent); - - if (p == m_paths.rend()) - { - // no, we don't. add it - e.path_index = m_paths.size(); - m_paths.push_back(parent); - } - else - { - // yes we do. use it - e.path_index = p.base() - m_paths.begin() - 1; - } - e.set_name(filename(e.filename()).c_str()); + e.no_root_dir = true; } + + // do we already have this path in the path list? + std::vector::reverse_iterator p + = std::find(m_paths.rbegin(), m_paths.rend(), parent); + + if (p == m_paths.rend()) + { + // no, we don't. add it + e.path_index = m_paths.size(); + m_paths.push_back(parent); + } + else + { + // yes we do. use it + e.path_index = p.base() - m_paths.begin() - 1; + } + e.set_name(filename(e.filename()).c_str()); } file_entry::file_entry(): offset(0), size(0), file_base(0) @@ -111,7 +129,10 @@ namespace libtorrent file_entry::~file_entry() {} - internal_file_entry::~internal_file_entry() { if (name_len == name_is_owned) free((void*)name); } + internal_file_entry::~internal_file_entry() + { + if (name_len == name_is_owned) free((void*)name); + } internal_file_entry::internal_file_entry(internal_file_entry const& fe) : name(0) @@ -123,6 +144,7 @@ namespace libtorrent , hidden_attribute(fe.hidden_attribute) , executable_attribute(fe.executable_attribute) , symlink_attribute(fe.symlink_attribute) + , no_root_dir(fe.no_root_dir) , path_index(fe.path_index) { set_name(fe.filename().c_str()); @@ -138,6 +160,7 @@ namespace libtorrent hidden_attribute = fe.hidden_attribute; executable_attribute = fe.executable_attribute; symlink_attribute = fe.symlink_attribute; + no_root_dir = fe.no_root_dir; set_name(fe.filename().c_str()); return *this; } @@ -332,7 +355,8 @@ namespace libtorrent ret.hidden_attribute = ife.hidden_attribute; ret.executable_attribute = ife.executable_attribute; ret.symlink_attribute = ife.symlink_attribute; - if (ife.symlink_index != internal_file_entry::not_a_symlink) ret.symlink_path = symlink(index); + if (ife.symlink_index != internal_file_entry::not_a_symlink) + ret.symlink_path = symlink(index); ret.filehash = hash(index); return ret; } @@ -494,14 +518,7 @@ namespace libtorrent { TORRENT_ASSERT(index >= 0 && index < int(m_files.size())); internal_file_entry const& fe = m_files[index]; - TORRENT_ASSERT(fe.path_index >= -2 && fe.path_index < int(m_paths.size())); - // -2 means this is an absolute path filename - if (fe.path_index == -2) return fe.filename(); - - // -1 means no path - if (fe.path_index == -1) return combine_path(save_path, fe.filename()); - - return combine_path(save_path, combine_path(m_paths[fe.path_index], fe.filename())); + return file_path(fe, save_path); } std::string file_storage::file_name(int index) const @@ -581,7 +598,8 @@ namespace libtorrent return m_file_base[index]; } - std::string file_storage::file_path(internal_file_entry const& fe, std::string const& save_path) const + std::string file_storage::file_path(internal_file_entry const& fe + , std::string const& save_path) const { TORRENT_ASSERT(fe.path_index >= -2 && fe.path_index < int(m_paths.size())); // -2 means this is an absolute path filename @@ -590,7 +608,15 @@ namespace libtorrent // -1 means no path if (fe.path_index == -1) return combine_path(save_path, fe.filename()); - return combine_path(save_path, combine_path(m_paths[fe.path_index], fe.filename())); + if (fe.no_root_dir) + return combine_path(save_path + , combine_path(m_paths[fe.path_index] + , fe.filename())); + + return combine_path(save_path + , combine_path(m_name + , combine_path(m_paths[fe.path_index] + , fe.filename()))); } std::string file_storage::file_name(internal_file_entry const& fe) const diff --git a/test/test_file_storage.cpp b/test/test_file_storage.cpp index f10ff2ec6..ed727ad7d 100644 --- a/test/test_file_storage.cpp +++ b/test/test_file_storage.cpp @@ -78,11 +78,13 @@ void setup_test_storage(file_storage& st) int test_main() { { + // test rename_file file_storage st; setup_test_storage(st); st.rename_file(0, combine_path("test", combine_path("c", "d"))); - TEST_EQUAL(st.file_path(0, "."), combine_path(".", combine_path("test", combine_path("c", "d")))); + TEST_EQUAL(st.file_path(0, "."), combine_path(".", combine_path("test" + , combine_path("c", "d")))); #ifdef TORRENT_WINDOWS st.rename_file(0, "c:\\tmp\\a"); @@ -93,6 +95,16 @@ int test_main() #endif } + { + // test set_name + file_storage st; + setup_test_storage(st); + + st.set_name("test_2"); + TEST_EQUAL(st.file_path(0, "."), combine_path(".", combine_path("test_2" + , "a"))); + } + { file_storage st; st.add_file("a", 10000);