From bc9281a27cffc53645cccf6521d773e668039611 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Wed, 21 Jan 2015 15:46:12 +0000 Subject: [PATCH] deprecate file_entry (used by file_storage) and clean up the add_file overload that borrows memory (and improve implementation and documentation). The python bindings were updated to more closely match the c++ API (specifically actually using file_storage) --- bindings/python/src/create_torrent.cpp | 47 ++++-- bindings/python/src/torrent_info.cpp | 35 ++--- include/libtorrent/file_storage.hpp | 106 +++++++------ include/libtorrent/torrent_info.hpp | 3 +- src/create_torrent.cpp | 26 ++-- src/disk_io_thread.cpp | 1 - src/file_storage.cpp | 133 ++++++++--------- src/torrent_info.cpp | 197 ++++++++++++------------- src/web_peer_connection.cpp | 4 +- test/test_torrent_info.cpp | 39 +++-- test/test_torrent_parse.cpp | 28 ++-- test/web_seed_suite.cpp | 4 +- 12 files changed, 324 insertions(+), 299 deletions(-) diff --git a/bindings/python/src/create_torrent.cpp b/bindings/python/src/create_torrent.cpp index 7770ace53..dd22e9053 100644 --- a/bindings/python/src/create_torrent.cpp +++ b/bindings/python/src/create_torrent.cpp @@ -54,10 +54,12 @@ namespace ct.add_node(std::make_pair(addr, port)); } +#if !defined TORRENT_NO_DEPRECATE void add_file(file_storage& ct, file_entry const& fe) { ct.add_file(fe); } +#endif char const* filestorage_name(file_storage const& fs) { return fs.name().c_str(); } @@ -76,10 +78,14 @@ namespace void bind_create_torrent() { - void (file_storage::*add_file0)(std::string const&, boost::int64_t, int, std::time_t, std::string const&) = &file_storage::add_file; -#if TORRENT_USE_WSTRING && !defined TORRENT_NO_DEPRECATE - void (file_storage::*add_file1)(std::wstring const&, boost::int64_t, int, std::time_t, std::string const&) = &file_storage::add_file; -#endif + void (file_storage::*add_file0)(std::string const&, boost::int64_t + , int, std::time_t, std::string const&) = &file_storage::add_file; +#if !defined TORRENT_NO_DEPRECATE +#if TORRENT_USE_WSTRING + void (file_storage::*add_file1)(std::wstring const&, boost::int64_t + , int, std::time_t, std::string const&) = &file_storage::add_file; +#endif // TORRENT_USE_WSTRING +#endif // TORRENT_NO_DEPRECATE void (file_storage::*set_name0)(std::string const&) = &file_storage::set_name; void (file_storage::*rename_file0)(int, std::string const&) = &file_storage::rename_file; @@ -93,23 +99,37 @@ void bind_create_torrent() #endif void (*add_files0)(file_storage&, std::string const&, boost::uint32_t) = add_files; + std::string const& (file_storage::*file_storage_symlink)(int) const = &file_storage::symlink; + sha1_hash (file_storage::*file_storage_hash)(int) const = &file_storage::hash; + std::string (file_storage::*file_storage_file_path)(int, std::string const&) const = &file_storage::file_path; + boost::int64_t (file_storage::*file_storage_file_size)(int) const = &file_storage::file_size; + boost::int64_t (file_storage::*file_storage_file_offset)(int) const = &file_storage::file_offset; + int (file_storage::*file_storage_file_flags)(int) const = &file_storage::file_flags; + +#if !defined TORRENT_NO_DEPRECATE file_entry (file_storage::*at)(int) const = &file_storage::at; +#endif class_("file_storage") .def("is_valid", &file_storage::is_valid) - .def("add_file", add_file, arg("entry")) .def("add_file", add_file0, (arg("path"), arg("size"), arg("flags") = 0, arg("mtime") = 0, arg("linkpath") = "")) #if TORRENT_USE_WSTRING && !defined TORRENT_NO_DEPRECATE .def("add_file", add_file1, (arg("path"), arg("size"), arg("flags") = 0, arg("mtime") = 0, arg("linkpath") = "")) #endif .def("num_files", &file_storage::num_files) +#if !defined TORRENT_NO_DEPRECATE .def("at", at) -// .def("hash", &file_storage::hash) -// .def("symlink", &file_storage::symlink, return_internal_reference<>()) -// .def("file_index", &file_storage::file_index) + .def("add_file", add_file, arg("entry")) +#endif + .def("hash", file_storage_hash) + .def("symlink", file_storage_symlink, return_value_policy()) // .def("file_base", &file_storage::file_base) // .def("set_file_base", &file_storage::set_file_base) -// .def("file_path", &file_storage::file_path) + .def("file_path", file_storage_file_path, (arg("idx"), arg("save_path") = "")) + .def("file_size", file_storage_file_size) + .def("file_offset", file_storage_file_offset) + .def("file_flags", file_storage_file_flags) + .def("total_size", &file_storage::total_size) .def("set_num_pieces", &file_storage::set_num_pieces) .def("num_pieces", &file_storage::num_pieces) @@ -122,9 +142,16 @@ void bind_create_torrent() .def("set_name", set_name1) .def("rename_file", rename_file1) #endif - .def("name", &filestorage_name) + .def("name", &file_storage::name, return_value_policy()) ; + enum_("file_flags_t") + .value("flag_pad_file", file_storage::flag_pad_file) + .value("flag_hidden", file_storage::flag_hidden) + .value("flag_executable", file_storage::flag_executable) + .value("flag_symlink", file_storage::flag_symlink) + ; + class_("create_torrent", no_init) .def(init()) .def(init(arg("ti"))) diff --git a/bindings/python/src/torrent_info.cpp b/bindings/python/src/torrent_info.cpp index 58838eb6b..44dba88fa 100644 --- a/bindings/python/src/torrent_info.cpp +++ b/bindings/python/src/torrent_info.cpp @@ -96,14 +96,7 @@ namespace } #endif - void remap_files(torrent_info& ti, list files) { - file_storage st; - for (int i = 0, e = len(files); i < e; ++i) - st.add_file(extract(files[i])); - - ti.remap_files(st); - } - +#if !defined TORRENT_NO_DEPRECATE list files(torrent_info const& ti, bool storage) { list result; @@ -123,6 +116,7 @@ namespace return result; } +#endif std::string hash_for_piece(torrent_info const& ti, int i) { @@ -157,7 +151,7 @@ namespace bool get_complete_sent(announce_entry const& ae) { return ae.complete_sent; } bool get_send_stats(announce_entry const& ae) { return ae.send_stats; } - +#if !defined TORRENT_NO_DEPRECATE boost::int64_t get_size(file_entry const& fe) { return fe.size; } boost::int64_t get_offset(file_entry const& fe) { return fe.offset; } boost::int64_t get_file_base(file_entry const& fe) { return fe.file_base; } @@ -166,6 +160,7 @@ namespace bool get_executable_attribute(file_entry const& fe) { return fe.executable_attribute; } bool get_hidden_attribute(file_entry const& fe) { return fe.hidden_attribute; } bool get_symlink_attribute(file_entry const& fe) { return fe.symlink_attribute; } +#endif } // namespace unnamed @@ -220,7 +215,6 @@ void bind_torrent_info() .def(init((arg("file"), arg("flags") = 0))) #endif - .def("remap_files", &remap_files) .def("add_tracker", &torrent_info::add_tracker, arg("url")) .def("add_url_seed", &torrent_info::add_url_seed) .def("add_http_seed", &torrent_info::add_http_seed) @@ -233,22 +227,23 @@ void bind_torrent_info() .def("piece_length", &torrent_info::piece_length) .def("num_pieces", &torrent_info::num_pieces) .def("info_hash", &torrent_info::info_hash, copy) -#ifndef TORRENT_NO_DEPRECATE - .def("file_at_offset", &torrent_info::file_at_offset) -#endif .def("hash_for_piece", &hash_for_piece) .def("merkle_tree", get_merkle_tree) .def("set_merkle_tree", set_merkle_tree) .def("piece_size", &torrent_info::piece_size) - .def("num_files", &torrent_info::num_files, (arg("storage")=false)) - .def("file_at", &torrent_info::file_at) - .def("files", &files, (arg("storage")=false)) - .def("orig_files", &orig_files, (arg("storage")=false)) + .def("num_files", &torrent_info::num_files) .def("rename_file", rename_file0) -#if TORRENT_USE_WSTRING && !defined TORRENT_NO_DEPRECATE + .def("remap_files", &torrent_info::remap_files) + .def("files", &torrent_info::files, return_internal_reference<>()) + .def("orig_files", &torrent_info::orig_files, return_internal_reference<>()) +#if !defined TORRENT_NO_DEPRECATE + .def("file_at", &torrent_info::file_at) + .def("file_at_offset", &torrent_info::file_at_offset) +#if TORRENT_USE_WSTRING .def("rename_file", rename_file1) -#endif +#endif // TORRENT_USE_WSTRING +#endif // TORRENT_NO_DEPRECATE .def("priv", &torrent_info::priv) .def("trackers", range(begin_trackers, end_trackers)) @@ -263,6 +258,7 @@ void bind_torrent_info() .def("map_file", &torrent_info::map_file) ; +#if !defined TORRENT_NO_DEPRECATE class_("file_entry") .def_readwrite("path", &file_entry::path) .def_readwrite("symlink_path", &file_entry::symlink_path) @@ -276,6 +272,7 @@ void bind_torrent_info() .add_property("size", &get_size) .add_property("file_base", &get_file_base, &set_file_base) ; +#endif class_("announce_entry", init()) .def_readwrite("url", &announce_entry::url) diff --git a/include/libtorrent/file_storage.hpp b/include/libtorrent/file_storage.hpp index 72ab969e4..3fc8c0c16 100644 --- a/include/libtorrent/file_storage.hpp +++ b/include/libtorrent/file_storage.hpp @@ -46,9 +46,7 @@ namespace libtorrent { struct file; - // TODO: 3 the file_entry should be deprecated and add_file() should be - // thought through a bit better - +#ifndef TORRENT_NO_DEPRECATE // information about a file in a file_storage struct TORRENT_EXPORT file_entry { @@ -106,6 +104,7 @@ namespace libtorrent // where the data for this file was found. bool symlink_attribute:1; }; +#endif // TORRENT_NO_DEPRECATE // internal struct TORRENT_DEPRECATED_EXPORT internal_file_entry @@ -130,22 +129,6 @@ namespace libtorrent , path_index(-1) {} - internal_file_entry(file_entry const& e) - : offset(e.offset) - , symlink_index(not_a_symlink) - , no_root_dir(false) - , size(e.size) - , 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) - , name(NULL) - , path_index(-1) - { - set_name(e.path.c_str()); - } - internal_file_entry(internal_file_entry const& fe); internal_file_entry& operator=(internal_file_entry const& fe); @@ -265,11 +248,35 @@ namespace libtorrent // of files to be added is known up-front. void reserve(int num_files); - // Adds a file to the file storage. The ``flags`` argument sets - // attributes on the file. The file attributes is an extension and may - // not work in all bittorrent clients. + // Adds a file to the file storage. The ``add_file_borrow`` version + // expects that ``filename`` points to a string of ``filename_len`` + // bytes that is the file name (without a path) of the file that's + // being added. This memory is *borrowed*, i.e. it is the caller's + // responsibility to make sure it stays valid throughout the lifetime + // of this file_storage object or any copy of it. The same thing applies + // to ``filehash``, wich is an optional pointer to a 20 byte binary + // SHA-1 hash of the file. + // + // if ``filename`` is NULL, the filename from ``path`` is used and not + // borrowed. In this case ``filename_len`` is ignored. + // + // The ``path`` argument is the full path (in the torrent file) to + // the file to add. Note that this is not supposed to be an absolute + // path, but it is expected to include the name of the torrent as the + // first path element. + // + // ``file_size`` is the size of the file in bytes. + // + // The ``file_flags`` argument sets attributes on the file. The file + // attributes is an extension and may not work in all bittorrent clients. // // For possible file attributes, see file_storage::flags_t. + // + // The ``mtime`` argument is optional and can be set to 0. If non-zero, + // it is the posix time of the last modification time of this file. + // + // ``symlink_path`` is the path the is a symlink to. To make this a + // symlink you also need to set the file_storage::flag_symlink file flag. // // If more files than one are added, certain restrictions to their paths // apply. In a multi-file file storage (torrent), all files must share @@ -278,16 +285,12 @@ namespace libtorrent // That is, the first path element of all files must be the same. // This shared path element is also set to the name of the torrent. It // can be changed by calling ``set_name``. - // - // The ``filehash`` argument is an optional pointer to a sha-1 hash (20 - // bytes) of the file. The hash is not copied into the file_storage - // object, but the pointer is expected to point to memory that stays - // valid throughout the life time of the file_storage. - // - // Currently, the ``filehash`` from ``file_entry`` is not used. - void add_file(file_entry const& e, char const* filehash = 0); - void add_file(std::string const& p, boost::int64_t size, int flags = 0 - , std::time_t mtime = 0, std::string const& s_p = ""); + void add_file_borrow(char const* filename, int filename_len + , std::string const& path, boost::int64_t file_size + , boost::uint32_t file_flags = 0, char const* filehash = 0 + , boost::int64_t mtime = 0, std::string const& symlink_path = ""); + void add_file(std::string const& path, boost::int64_t file_size, int file_flags = 0 + , std::time_t mtime = 0, std::string const& symlink_path = ""); // renames the file at ``index`` to ``new_filename``. Keep in mind // that filenames are expected to be UTF-8 encoded. @@ -299,11 +302,14 @@ namespace libtorrent // duplicate names in memory. void rename_file_borrow(int index, char const* new_filename, int len); +#ifndef TORRENT_NO_DEPRECATE + TORRENT_DEPRECATED_PREFIX + void add_file(file_entry const& fe, char const* infohash = NULL) TORRENT_DEPRECATED; + #if TORRENT_USE_WSTRING // all wstring APIs are deprecated since 0.16.11 // instead, use the wchar -> utf8 conversion functions // and pass in utf8 strings -#ifndef TORRENT_NO_DEPRECATE TORRENT_DEPRECATED_PREFIX void add_file(std::wstring const& p, boost::int64_t size, int flags = 0 , std::time_t mtime = 0, std::string const& s_p = "") TORRENT_DEPRECATED; @@ -313,8 +319,8 @@ namespace libtorrent void set_name(std::wstring const& n) TORRENT_DEPRECATED; void rename_file_deprecated(int index, std::wstring const& new_filename); -#endif // TORRENT_NO_DEPRECATE #endif // TORRENT_USE_WSTRING +#endif // TORRENT_NO_DEPRECATE // returns a list of file_slice objects representing the portions of // files the specified piece index, byte offset and size range overlaps. @@ -354,6 +360,11 @@ namespace libtorrent TORRENT_DEPRECATED_PREFIX file_entry at(iterator i) const TORRENT_DEPRECATED; + // returns a file_entry with information about the file + // at ``index``. Index must be in the range [0, ``num_files()`` ). + TORRENT_DEPRECATED_PREFIX + file_entry at(int index) const TORRENT_DEPRECATED; + iterator begin_deprecated() const { return m_files.begin(); } iterator end_deprecated() const { return m_files.end(); } reverse_iterator rbegin_deprecated() const { return m_files.rbegin(); } @@ -365,10 +376,6 @@ namespace libtorrent int num_files() const { return int(m_files.size()); } - // returns a file_entry with information about the file - // at ``index``. Index must be in the range [0, ``num_files()`` ). - file_entry at(int index) const; - // returns the total number of bytes all the files in this torrent spans boost::int64_t total_size() const { return m_total_size; } @@ -388,7 +395,7 @@ namespace libtorrent // set and get the name of this torrent. For multi-file torrents, this is also // the name of the root directory all the files are stored in. void set_name(std::string const& n) { m_name = n; } - const std::string& name() const { return m_name; } + std::string const& name() const { return m_name; } // swap all content of *this* with *ti*. void swap(file_storage& ti) @@ -487,9 +494,9 @@ namespace libtorrent int file_flags(int index) const; // The file base of a file is the offset within the file on the filsystem - // where it starts to write. For the most part, this is always 0. It's - // possible to map several files (in the torrent) into a single file on - // the filesystem by making them all point to the same filename, but with + // where it starts to write. For the most part, this is 0. It's possible + // to map several files (in the torrent) into a single file on the + // filesystem by making them all point to the same filename, but with // different file bases, so that they don't overlap. // torrent_info::remap_files() can be used to use a new file layout. boost::int64_t file_base(int index) const; @@ -549,12 +556,15 @@ namespace libtorrent // the list of files that this torrent consists of std::vector m_files; - // if there are sha1 hashes for each individual file - // there are as many entries in this array as the - // m_files array. Each entry in m_files has a corresponding - // hash pointer in this array. The reason to split it up - // in separate arrays is to save memory in case the torrent - // doesn't have file hashes + // if there are sha1 hashes for each individual file there are as many + // entries in this array as the m_files array. Each entry in m_files has + // a corresponding hash pointer in this array. The reason to split it up + // in separate arrays is to save memory in case the torrent doesn't have + // file hashes + // the pointers in this vector are pointing into the .torrent file in + // memory which is _not_ owned by this file_storage object. It's simply + // a non-owning pointer. It is the user's responsibility that the hash + // stays valid throughout the lifetime of this file_storage object. std::vector m_file_hashes; // for files that are symlinks, the symlink diff --git a/include/libtorrent/torrent_info.hpp b/include/libtorrent/torrent_info.hpp index 37151145d..e2a01cf8e 100644 --- a/include/libtorrent/torrent_info.hpp +++ b/include/libtorrent/torrent_info.hpp @@ -483,12 +483,13 @@ namespace libtorrent file_iterator file_at_offset(boost::int64_t offset) const TORRENT_DEPRECATED { return m_files.file_at_offset_deprecated(offset); } + TORRENT_DEPRECATED_PREFIX + file_entry file_at(int index) const TORRENT_DEPRECATED{ return m_files.at(index); } #endif // TORRENT_NO_DEPRECATE // If you need index-access to files you can use the ``num_files()`` and // ``file_at()`` to access files using indices. int num_files() const { return m_files.num_files(); } - file_entry file_at(int index) const { return m_files.at(index); } // This function will map a piece index, a byte offset within that piece // and a size (in bytes) into the corresponding files with offsets where diff --git a/src/create_torrent.cpp b/src/create_torrent.cpp index 10a95f241..6da0ef735 100644 --- a/src/create_torrent.cpp +++ b/src/create_torrent.cpp @@ -490,26 +490,26 @@ namespace libtorrent if (!m_multifile) { - file_entry e = m_files.at(0); - if (m_include_mtime) info["mtime"] = e.mtime; - info["length"] = e.size; - if (e.pad_file - || e.hidden_attribute - || e.executable_attribute - || e.symlink_attribute) + if (m_include_mtime) info["mtime"] = m_files.mtime(0); + info["length"] = m_files.file_size(0); + int flags = m_files.file_flags(0); + if (flags & (file_storage::flag_pad_file + | file_storage::flag_hidden + | file_storage::flag_executable + | file_storage::flag_symlink)) { std::string& attr = info["attr"].string(); - if (e.pad_file) attr += 'p'; - if (e.hidden_attribute) attr += 'h'; - if (e.executable_attribute) attr += 'x'; - if (m_include_symlinks && e.symlink_attribute) attr += 'l'; + if (flags & file_storage::flag_pad_file) attr += 'p'; + if (flags & file_storage::flag_hidden) attr += 'h'; + if (flags & file_storage::flag_executable) attr += 'x'; + if (m_include_symlinks && (flags & file_storage::flag_symlink)) attr += 'l'; } if (m_include_symlinks - && e.symlink_attribute) + && (flags & file_storage::flag_symlink)) { entry& sympath_e = info["symlink path"]; - std::string split = split_path(e.symlink_path); + std::string split = split_path(m_files.symlink(0)); for (char const* e = split.c_str(); e != 0; e = next_path_element(e)) sympath_e.list().push_back(entry(e)); } diff --git a/src/disk_io_thread.cpp b/src/disk_io_thread.cpp index aa55f871a..930c9ef14 100644 --- a/src/disk_io_thread.cpp +++ b/src/disk_io_thread.cpp @@ -835,7 +835,6 @@ namespace libtorrent if (storage) { boost::unordered_set const& pieces = storage->cached_pieces(); - // TODO: 2 should this be allocated on the stack? std::vector piece_index; piece_index.reserve(pieces.size()); for (boost::unordered_set::const_iterator i = pieces.begin() diff --git a/src/file_storage.cpp b/src/file_storage.cpp index e7f657f5e..5e835387e 100644 --- a/src/file_storage.cpp +++ b/src/file_storage.cpp @@ -132,6 +132,7 @@ namespace libtorrent e.set_name(leaf); } +#ifndef TORRENT_NO_DEPRECATE file_entry::file_entry(): offset(0), size(0), file_base(0) , mtime(0), pad_file(false), hidden_attribute(false) , executable_attribute(false) @@ -139,6 +140,7 @@ namespace libtorrent {} file_entry::~file_entry() {} +#endif // TORRENT_NO_DEPRECATE internal_file_entry::~internal_file_entry() { @@ -231,8 +233,21 @@ namespace libtorrent } } -#if TORRENT_USE_WSTRING #ifndef TORRENT_NO_DEPRECATE + + void file_storage::add_file(file_entry const& fe, char const* infohash) + { + int flags = 0; + if (fe.pad_file) flags |= file_storage::flag_pad_file; + if (fe.hidden_attribute) flags |= file_storage::flag_hidden; + if (fe.executable_attribute) flags |= file_storage::flag_executable; + if (fe.symlink_attribute) flags |= file_storage::flag_symlink; + + add_file_borrow(NULL, 0, fe.path, fe.size, flags, NULL, fe.mtime + , fe.symlink_path); + } + +#if TORRENT_USE_WSTRING void file_storage::set_name(std::wstring const& n) { std::string utf8; @@ -249,20 +264,20 @@ namespace libtorrent update_path_index(m_files[index]); } - void file_storage::add_file(std::wstring const& file, boost::int64_t size, int flags - , std::time_t mtime, std::string const& symlink_path) + void file_storage::add_file(std::wstring const& file, boost::int64_t file_size + , int file_flags, std::time_t mtime, std::string const& symlink_path) { std::string utf8; wchar_utf8(file, utf8); - add_file(utf8, size, flags, mtime, symlink_path); + add_file(utf8, file_size, file_flags, mtime, symlink_path); } void file_storage::rename_file(int index, std::wstring const& new_filename) { rename_file_deprecated(index, new_filename); } -#endif // TORRENT_NO_DEPRECATE #endif // TORRENT_USE_WSTRING +#endif // TORRENT_NO_DEPRECATE void file_storage::rename_file(int index, std::string const& new_filename) { @@ -375,6 +390,7 @@ namespace libtorrent return ret; } +#ifndef TORRENT_NO_DEPRECATE file_entry file_storage::at(int index) const { TORRENT_ASSERT_PRECOND(index >= 0 && index < int(m_files.size())); @@ -394,6 +410,7 @@ namespace libtorrent ret.filehash = hash(index); return ret; } +#endif // TORRENT_NO_DEPRECATE peer_request file_storage::map_file(int file_index, boost::int64_t file_offset , int size) const @@ -430,99 +447,77 @@ namespace libtorrent return ret; } - void file_storage::add_file(std::string const& file, boost::int64_t size, int flags - , std::time_t mtime, std::string const& symlink_path) + void file_storage::add_file(std::string const& path, boost::int64_t file_size + , int file_flags, std::time_t mtime, std::string const& symlink_path) { - TORRENT_ASSERT_PRECOND(!is_complete(file)); - TORRENT_ASSERT_PRECOND(size >= 0); - if (size < 0) size = 0; - if (!has_parent_path(file)) - { - // you have already added at least one file with a - // path to the file (branch_path), which means that - // all the other files need to be in the same top - // directory as the first file. - TORRENT_ASSERT_PRECOND(m_files.empty()); - m_name = file; - } - else - { - if (m_files.empty()) - m_name = split_path(file).c_str(); - } - TORRENT_ASSERT_PRECOND(m_name == split_path(file).c_str()); - m_files.push_back(internal_file_entry()); - ++m_num_files; - internal_file_entry& e = m_files.back(); - e.set_name(file.c_str()); - e.size = size; - e.offset = m_total_size; - e.pad_file = (flags & pad_file) != 0; - e.hidden_attribute = (flags & attribute_hidden) != 0; - e.executable_attribute = (flags & attribute_executable) != 0; - if ((flags & attribute_symlink) && m_symlinks.size() < internal_file_entry::not_a_symlink - 1) - { - e.symlink_attribute = 1; - e.symlink_index = m_symlinks.size(); - m_symlinks.push_back(symlink_path); - } - else - e.symlink_attribute = 0; - - if (mtime) - { - if (m_mtime.size() < m_files.size()) m_mtime.resize(m_files.size()); - m_mtime[m_files.size() - 1] = mtime; - } - - update_path_index(e); - m_total_size += size; + add_file_borrow(NULL, 0, path, file_size, file_flags, NULL, mtime + , symlink_path); } - // TODO: 2 it would be nice if file_entry::filehash could be taken into - // account as well, and if the file_storage object could actually hold - // copies of filehash - void file_storage::add_file(file_entry const& ent, char const* filehash) + void file_storage::add_file_borrow(char const* filename, int filename_len + , std::string const& path, boost::int64_t file_size + , boost::uint32_t file_flags, char const* filehash + , boost::int64_t mtime, std::string const& symlink_path) { - TORRENT_ASSERT_PRECOND(ent.size >= 0); - if (!has_parent_path(ent.path)) + TORRENT_ASSERT_PRECOND(file_size >= 0); + if (!has_parent_path(path)) { // you have already added at least one file with a // path to the file (branch_path), which means that // all the other files need to be in the same top // directory as the first file. TORRENT_ASSERT_PRECOND(m_files.empty()); - m_name = ent.path; + m_name = path; } else { if (m_files.empty()) - m_name = split_path(ent.path).c_str(); + m_name = split_path(path).c_str(); } - internal_file_entry ife(ent); - int file_index = m_files.size(); + internal_file_entry ife; m_files.push_back(ife); - ++m_num_files; internal_file_entry& e = m_files.back(); + + // first set the filename to the full path so update_path_index() + // can do its thing, then rename it to point to the borrowed filename + // pointer + e.set_name(path.c_str()); + update_path_index(e); + + // filename is allowed to be NULL, in which case we just use path + if (filename) + e.set_name(filename, true, filename_len); + + e.size = file_size; e.offset = m_total_size; - m_total_size += e.size; + e.pad_file = file_flags & file_storage::flag_pad_file; + e.hidden_attribute = file_flags & file_storage::flag_hidden; + e.executable_attribute = file_flags & file_storage::flag_executable; + e.symlink_attribute = file_flags & file_storage::flag_symlink; + if (filehash) { if (m_file_hashes.size() < m_files.size()) m_file_hashes.resize(m_files.size()); m_file_hashes[m_files.size() - 1] = filehash; } - if (!ent.symlink_path.empty() && m_symlinks.size() < internal_file_entry::not_a_symlink - 1) + if (!symlink_path.empty() + && m_symlinks.size() < internal_file_entry::not_a_symlink - 1) { e.symlink_index = m_symlinks.size(); - m_symlinks.push_back(ent.symlink_path); + m_symlinks.push_back(symlink_path); } - if (ent.mtime) + else + { + e.symlink_attribute = false; + } + if (mtime) { if (m_mtime.size() < m_files.size()) m_mtime.resize(m_files.size()); - m_mtime[m_files.size() - 1] = ent.mtime; + m_mtime[m_files.size() - 1] = mtime; } - if (ent.file_base) set_file_base(file_index, ent.file_base); - update_path_index(e); + + ++m_num_files; + m_total_size += e.size; } sha1_hash file_storage::hash(int index) const diff --git a/src/torrent_info.cpp b/src/torrent_info.cpp index c6d6a539a..23ce10da2 100644 --- a/src/torrent_info.cpp +++ b/src/torrent_info.cpp @@ -357,47 +357,77 @@ namespace libtorrent if (path.empty()) path = "_"; } - bool extract_single_file(lazy_entry const& dict, file_entry& target - , std::string const& root_dir, lazy_entry const** filehash - , lazy_entry const** filename, time_t* mtime) + // top level is extracting the file for a single-file torrent. The + // distinction is that the filename is found in "name" rather than + // "path" + bool extract_single_file(lazy_entry const& dict, file_storage& files + , std::string const& root_dir, ptrdiff_t info_ptr_diff, bool top_level + , error_code& ec) { if (dict.type() != lazy_entry::dict_t) return false; - lazy_entry const* length = dict.dict_find("length"); - if (length == 0 || length->type() != lazy_entry::int_t) - return false; - target.size = length->int_value(); - if (target.size < 0) + boost::int64_t file_size = dict.dict_find_int_value("length", -1); + if (file_size < 0) + { + ec = errors::torrent_invalid_length; return false; + } - boost::int64_t ts = dict.dict_find_int_value("mtime", -1); - if (ts > 0) *mtime = std::time_t(ts); - - // prefer the name.utf-8 - // because if it exists, it is more - // likely to be correctly encoded - - lazy_entry const* p = dict.dict_find("path.utf-8"); - if (p == 0 || p->type() != lazy_entry::list_t) - p = dict.dict_find("path"); - if (p == 0 || p->type() != lazy_entry::list_t) - return false; + boost::int64_t mtime = dict.dict_find_int_value("mtime", 0); + // prefer the name.utf-8 because if it exists, it is more likely to be + // correctly encoded std::string path = root_dir; std::string path_element; - for (int i = 0, end(p->list_size()); i < end; ++i) + char const* filename = NULL; + int filename_len = 0; + + if (top_level) { - if (p->list_at(i)->type() != lazy_entry::string_t) + lazy_entry const* p = dict.dict_find_string("name.utf-8"); + if (p == 0) p = dict.dict_find_string("name"); + if (p == 0 || p->string_length() == 0) + { + ec = errors::torrent_missing_name; return false; - if (i == end - 1) *filename = p->list_at(i); - sanitize_append_path_element(path - , p->list_at(i)->string_ptr(), p->list_at(i)->string_length()); + } + + filename = p->string_ptr() + info_ptr_diff; + filename_len = p->string_length(); + sanitize_append_path_element(path, p->string_ptr(), p->string_length()); + +// if (path.empty()) path = to_hex(files.info_hash().to_string()); + } + else + { + lazy_entry const* p = dict.dict_find_list("path.utf-8"); + if (p == 0) p = dict.dict_find_list("path"); + if (p == 0 || p->list_size() == 0) + { + ec = errors::torrent_missing_name; + return false; + } + + for (int i = 0, end(p->list_size()); i < end; ++i) + { + lazy_entry const* e = p->list_at(i); + if (e->type() != lazy_entry::string_t) + { + ec = errors::torrent_missing_name; + return false; + } + if (i == end - 1) + { + filename = e->string_ptr() + info_ptr_diff; + filename_len = e->string_length(); + } + sanitize_append_path_element(path, e->string_ptr(), e->string_length()); + } } // bitcomet pad file + boost::uint32_t file_flags = 0; if (path.find("_____padding_file_") != std::string::npos) - target.pad_file = true; - - target.path = path; + file_flags = file_storage::flag_pad_file; lazy_entry const* attr = dict.dict_find_string("attr"); if (attr) @@ -406,32 +436,45 @@ namespace libtorrent { switch (attr->string_ptr()[i]) { - case 'l': target.symlink_attribute = true; target.size = 0; break; - case 'x': target.executable_attribute = true; break; - case 'h': target.hidden_attribute = true; break; - case 'p': target.pad_file = true; break; + case 'l': file_flags |= file_storage::flag_symlink; file_size = 0; break; + case 'x': file_flags |= file_storage::flag_executable; break; + case 'h': file_flags |= file_storage::flag_hidden; break; + case 'p': file_flags |= file_storage::flag_pad_file; break; } } } lazy_entry const* fh = dict.dict_find_string("sha1"); - if (fh && fh->string_length() == 20 && filehash) - *filehash = fh; + char const* filehash = NULL; + if (fh && fh->string_length() == 20) + filehash = fh->string_ptr() + info_ptr_diff; + std::string symlink_path; lazy_entry const* s_p = dict.dict_find("symlink path"); - if (s_p != 0 && s_p->type() == lazy_entry::list_t && target.symlink_attribute) + if (s_p != 0 && s_p->type() == lazy_entry::list_t + && (file_flags & file_storage::flag_symlink)) { for (int i = 0, end(s_p->list_size()); i < end; ++i) { std::string path_element = s_p->list_at(i)->string_value(); - target.symlink_path = combine_path(target.symlink_path, path_element); + symlink_path = combine_path(symlink_path, path_element); } } else { - target.symlink_attribute = false; + file_flags &= ~file_storage::flag_symlink; } + if (filename_len > path.length() + || path.compare(path.size() - filename_len, filename_len, filename) != 0) + { + // if the filename was sanitized and differ, clear it to just use path + filename = NULL; + filename_len = 0; + } + + files.add_file_borrow(filename, filename_len, path, file_size, file_flags, filehash + , mtime, symlink_path); return true; } @@ -495,32 +538,20 @@ namespace libtorrent #endif bool extract_files(lazy_entry const& list, file_storage& target - , std::string const& root_dir, ptrdiff_t info_ptr_diff) + , std::string const& root_dir, ptrdiff_t info_ptr_diff, error_code& ec) { - if (list.type() != lazy_entry::list_t) return false; + if (list.type() != lazy_entry::list_t) + { + ec = errors::torrent_file_parse_failed; + return false; + } target.reserve(list.list_size()); for (int i = 0, end(list.list_size()); i < end; ++i) { - lazy_entry const* file_hash = 0; - time_t mtime = 0; - file_entry e; - lazy_entry const* fee = 0; - if (!extract_single_file(*list.list_at(i), e, root_dir - , &file_hash, &fee, &mtime)) + if (!extract_single_file(*list.list_at(i), target, root_dir + , info_ptr_diff, false, ec)) return false; - - target.add_file(e, file_hash ? file_hash->string_ptr() + info_ptr_diff : 0); - - // This is a memory optimization! Instead of having - // each entry keep a string for its filename, make it - // simply point into the info-section buffer - int last_index = target.num_files() - 1; - - // this string pointer does not necessarily point into - // the m_info_section buffer. - char const* str_ptr = fee->string_ptr() + info_ptr_diff; - target.rename_file_borrow(last_index, str_ptr, fee->string_length()); } return true; } @@ -565,7 +596,7 @@ namespace libtorrent if (ec) return -1; if (s > limit) { - ec = error_code(errors::metadata_too_large, get_libtorrent_category()); + ec = errors::metadata_too_large; return -2; } v.resize((unsigned int)s); @@ -1151,61 +1182,15 @@ namespace libtorrent { // if there's no list of files, there has to be a length // field. - file_entry e; - e.path = name; - e.offset = 0; - e.size = info.dict_find_int_value("length", -1); - if (e.size < 0) - { - ec = errors::torrent_invalid_length; + if (!extract_single_file(info, files, "", info_ptr_diff, true, ec)) return false; - } - e.mtime = info.dict_find_int_value("mtime", 0); - lazy_entry const* attr = info.dict_find_string("attr"); - if (attr) - { - for (int i = 0; i < attr->string_length(); ++i) - { - switch (attr->string_ptr()[i]) - { - case 'l': e.symlink_attribute = true; e.size = 0; break; - case 'x': e.executable_attribute = true; break; - case 'h': e.hidden_attribute = true; break; - case 'p': e.pad_file = true; break; - } - } - } - lazy_entry const* s_p = info.dict_find("symlink path"); - if (s_p != 0 && s_p->type() == lazy_entry::list_t) - { - for (int i = 0, end(s_p->list_size()); i < end; ++i) - { - std::string path_element = s_p->list_at(i)->string_value(); - e.symlink_path = combine_path(e.symlink_path, path_element); - } - } - else - { - e.symlink_attribute = false; - } - - lazy_entry const* fh = info.dict_find_string("sha1"); - if (fh && fh->string_length() != 20) fh = 0; - - // bitcomet pad file - if (e.path.find("_____padding_file_") != std::string::npos) - e.pad_file = true; - files.add_file(e, fh ? fh->string_ptr() + info_ptr_diff : 0); m_multifile = false; } else { - if (!extract_files(*i, files, name, info_ptr_diff)) - { - ec = errors::torrent_file_parse_failed; + if (!extract_files(*i, files, name, info_ptr_diff, ec)) return false; - } m_multifile = true; } TORRENT_ASSERT(!files.name().empty()); diff --git a/src/web_peer_connection.cpp b/src/web_peer_connection.cpp index 58041a5f0..6546fe62e 100644 --- a/src/web_peer_connection.cpp +++ b/src/web_peer_connection.cpp @@ -244,7 +244,7 @@ void web_peer_connection::write_request(peer_request const& r) if (m_path.empty()) m_path += "/" + t->torrent_file().name(); else if (m_path[m_path.size() - 1] == '/') { - std::string tmp = t->torrent_file().files().at(0).path; + std::string tmp = t->torrent_file().files().file_path(0); #ifdef TORRENT_WINDOWS convert_path_to_posix(tmp); #endif @@ -252,7 +252,7 @@ void web_peer_connection::write_request(peer_request const& r) } else if (!m_url.empty() && m_url[m_url.size() - 1] == '/') { - std::string tmp = t->torrent_file().files().at(0).path; + std::string tmp = t->torrent_file().files().file_path(0); #ifdef TORRENT_WINDOWS convert_path_to_posix(tmp); #endif diff --git a/test/test_torrent_info.cpp b/test/test_torrent_info.cpp index 4ffc6c462..f2507441b 100644 --- a/test/test_torrent_info.cpp +++ b/test/test_torrent_info.cpp @@ -85,10 +85,10 @@ void test_storage() for (int i = 0; i < ti.num_files(); ++i) { - std::string p = ti.file_at(i).path; + std::string p = ti.files().file_path(i); convert_path_to_posix(p); fprintf(stderr, "%s == %s\n", p.c_str(), filenames[i]); - TEST_CHECK(p == filenames[i]); + TEST_EQUAL(p, filenames[i]); } } @@ -100,17 +100,6 @@ void test_copy() combine_path(parent_path(current_working_directory()) , combine_path("test_torrents", "sample.torrent")))); - boost::shared_ptr b(boost::make_shared(*a)); - - // clear out the buffer for a, just to make sure b doesn't have any - // references into it by mistake - int s = a->metadata_size(); - memset(a->metadata().get(), 0, s); - - a.reset(); - - TEST_EQUAL(b->num_files(), 3); - char const* expected_files[] = { "sample/text_file2.txt", @@ -125,9 +114,31 @@ void test_copy() sha1_hash("abababababababababab") }; + for (int i = 0; i < a->num_files(); ++i) + { + std::string p = a->files().file_path(i); + convert_path_to_posix(p); + TEST_EQUAL(p, expected_files[i]); + fprintf(stderr, "%s\n", p.c_str()); + + TEST_EQUAL(a->files().hash(i), file_hashes[i]); + } + + // copy the torrent_info object + boost::shared_ptr b(boost::make_shared(*a)); + + // clear out the buffer for a, just to make sure b doesn't have any + // references into it by mistake + int s = a->metadata_size(); + memset(a->metadata().get(), 0, s); + + a.reset(); + + TEST_EQUAL(b->num_files(), 3); + for (int i = 0; i < b->num_files(); ++i) { - std::string p = b->file_at(i).path; + std::string p = b->files().file_path(i); convert_path_to_posix(p); TEST_EQUAL(p, expected_files[i]); fprintf(stderr, "%s\n", p.c_str()); diff --git a/test/test_torrent_parse.cpp b/test/test_torrent_parse.cpp index 4bdda1881..878175a24 100644 --- a/test/test_torrent_parse.cpp +++ b/test/test_torrent_parse.cpp @@ -96,14 +96,14 @@ test_failing_torrent_t test_error_torrents[] = { "invalid_info.torrent", errors::torrent_missing_info }, { "string.torrent", errors::torrent_is_no_dict }, { "negative_size.torrent", errors::torrent_invalid_length }, - { "negative_file_size.torrent", errors::torrent_file_parse_failed }, - { "invalid_path_list.torrent", errors::torrent_file_parse_failed }, - { "missing_path_list.torrent", errors::torrent_file_parse_failed }, + { "negative_file_size.torrent", errors::torrent_invalid_length }, + { "invalid_path_list.torrent", errors::torrent_missing_name}, + { "missing_path_list.torrent", errors::torrent_missing_name }, { "invalid_pieces.torrent", errors::torrent_missing_pieces }, { "unaligned_pieces.torrent", errors::torrent_invalid_hashes }, { "invalid_root_hash.torrent", errors::torrent_invalid_hashes }, - { "invalid_root_hash2.torrent", errors::torrent_missing_pieces}, - { "invalid_file_size.torrent", errors::torrent_file_parse_failed }, + { "invalid_root_hash2.torrent", errors::torrent_missing_pieces }, + { "invalid_file_size.torrent", errors::torrent_invalid_length }, }; namespace libtorrent @@ -377,14 +377,14 @@ int test_main() { // make sure we disambiguated the files TEST_EQUAL(ti->num_files(), 2); - TEST_CHECK(ti->file_at(0).path == combine_path(combine_path("temp", "foo"), "bar.txt")); - TEST_CHECK(ti->file_at(1).path == combine_path(combine_path("temp", "foo"), "bar.1.txt")); + TEST_CHECK(ti->files().file_path(0) == combine_path(combine_path("temp", "foo"), "bar.txt")); + TEST_CHECK(ti->files().file_path(1) == combine_path(combine_path("temp", "foo"), "bar.1.txt")); } else if (std::string(test_torrents[i].file) == "pad_file.torrent") { TEST_EQUAL(ti->num_files(), 2); - TEST_EQUAL(ti->file_at(0).pad_file, false); - TEST_EQUAL(ti->file_at(1).pad_file, true); + TEST_EQUAL(ti->files().file_flags(0) & file_storage::flag_pad_file, false); + TEST_EQUAL(ti->files().file_flags(1) & file_storage::flag_pad_file, true); } else if (std::string(test_torrents[i].file) == "creation_date.torrent") { @@ -444,24 +444,24 @@ int test_main() { TEST_EQUAL(ti->num_files(), 1); #ifdef TORRENT_WINDOWS - TEST_EQUAL(ti->file_at(0).path, "temp\\bar"); + TEST_EQUAL(ti->files().file_path(0), "temp\\bar"); #else - TEST_EQUAL(ti->file_at(0).path, "temp/bar"); + TEST_EQUAL(ti->files().file_path(0), "temp/bar"); #endif } else if (std::string(test_torrents[i].file) == "slash_path2.torrent") { TEST_EQUAL(ti->num_files(), 1); #ifdef TORRENT_WINDOWS - TEST_EQUAL(ti->file_at(0).path, "temp\\abc....def\\bar"); + TEST_EQUAL(ti->files().file_path(0), "temp\\abc....def\\bar"); #else - TEST_EQUAL(ti->file_at(0).path, "temp/abc....def/bar"); + TEST_EQUAL(ti->files().file_path(0), "temp/abc....def/bar"); #endif } else if (std::string(test_torrents[i].file) == "slash_path3.torrent") { TEST_EQUAL(ti->num_files(), 1); - TEST_EQUAL(ti->file_at(0).path, "temp....abc"); + TEST_EQUAL(ti->files().file_path(0), "temp....abc"); } file_storage const& fs = ti->files(); diff --git a/test/web_seed_suite.cpp b/test/web_seed_suite.cpp index 2c29e9d49..94afe8a68 100644 --- a/test/web_seed_suite.cpp +++ b/test/web_seed_suite.cpp @@ -155,8 +155,8 @@ void test_transfer(lt::session& ses, boost::shared_ptr torrent_fil int pad_file_size = 0; for (int i = 0; i < fs.num_files(); ++i) { - file_entry f = fs.at(i); - if (f.pad_file) pad_file_size += f.size; + if (fs.file_flags(i) & file_storage::flag_pad_file) + pad_file_size += fs.file_size(i); } peer_disconnects = 0;