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

@@ -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;
+    void (file_storage::*add_file0)(std::string const&, boost::int64_t
-#if TORRENT_USE_WSTRING && !defined TORRENT_NO_DEPRECATE
+		 , int, std::time_t, std::string const&) = &file_storage::add_file;
-    void (file_storage::*add_file1)(std::wstring const&, boost::int64_t, int, std::time_t, std::string const&) = &file_storage::add_file;
+#if !defined TORRENT_NO_DEPRECATE
-#endif
+#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>("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("add_file", add_file, arg("entry"))
-//        .def("symlink", &file_storage::symlink, return_internal_reference<>())
+#endif
-//        .def("file_index", &file_storage::file_index)
+        .def("hash", file_storage_hash)
+        .def("symlink", file_storage_symlink, return_value_policy<copy_const_reference>())
 //        .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<copy_const_reference>())
         ;
 
+    enum_<file_storage::file_flags_t>("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>("create_torrent", no_init)
         .def(init<file_storage&>())
         .def(init<torrent_info const&>(arg("ti")))

bindings/python/src/torrent_info.cpp

@@ -96,14 +96,7 @@ namespace
     }
 #endif
 
-    void remap_files(torrent_info& ti, list files) {
+#if !defined TORRENT_NO_DEPRECATE
-        file_storage st;
-        for (int i = 0, e = len(files); i < e; ++i)
-            st.add_file(extract<file_entry>(files[i]));
-
-        ti.remap_files(st);
-    }
-
     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<std::wstring, int>((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("num_files", &torrent_info::num_files)
-        .def("file_at", &torrent_info::file_at)
-        .def("files", &files, (arg("storage")=false))
-        .def("orig_files", &orig_files, (arg("storage")=false))
         .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>("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>("announce_entry", init<std::string const&>())
         .def_readwrite("url", &announce_entry::url)

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
+#ifndef TORRENT_NO_DEPRECATE
-	// thought through a bit better
-
 	// 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
+		// Adds a file to the file storage. The ``add_file_borrow`` version
-		// attributes on the file. The file attributes is an extension and may
+		// expects that ``filename`` points to a string of ``filename_len``
-		// not work in all bittorrent clients.
+		// 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``.
-		// 
+		void add_file_borrow(char const* filename, int filename_len
-		// The ``filehash`` argument is an optional pointer to a sha-1 hash (20
+			, std::string const& path, boost::int64_t file_size
-		// bytes) of the file. The hash is not copied into the file_storage
+			, boost::uint32_t file_flags = 0, char const* filehash = 0
-		// object, but the pointer is expected to point to memory that stays
+			, boost::int64_t mtime = 0, std::string const& symlink_path = "");
-		// valid throughout the life time of the file_storage.
+		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 = "");
-		// 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 = "");
 
 		// 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
+		// where it starts to write. For the most part, this is 0. It's possible
-		// possible to map several files (in the torrent) into a single file on
+		// to map several files (in the torrent) into a single file on the
-		// the filesystem by making them all point to the same filename, but with
+		// 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<internal_file_entry> m_files;
 
-		// if there are sha1 hashes for each individual file
+		// if there are sha1 hashes for each individual file there are as many
-		// there are as many entries in this array as the
+		// entries in this array as the m_files array. Each entry in m_files has
-		// m_files array. Each entry in m_files has a corresponding
+		// a corresponding hash pointer in this array. The reason to split it up
-		// 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
-		// in separate arrays is to save memory in case the torrent
+		// file hashes
-		// 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<char const*> m_file_hashes;
 
 		// for files that are symlinks, the symlink

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

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"] = m_files.mtime(0);
-			if (m_include_mtime) info["mtime"] = e.mtime;
+			info["length"] = m_files.file_size(0);
-			info["length"] = e.size;
+			int flags = m_files.file_flags(0);
-			if (e.pad_file
+			if (flags & (file_storage::flag_pad_file
-				|| e.hidden_attribute
+				| file_storage::flag_hidden
-				|| e.executable_attribute
+				| file_storage::flag_executable
-				|| e.symlink_attribute)
+				| file_storage::flag_symlink))
 			{
 				std::string& attr = info["attr"].string();
-				if (e.pad_file) attr += 'p';
+				if (flags & file_storage::flag_pad_file) attr += 'p';
-				if (e.hidden_attribute) attr += 'h';
+				if (flags & file_storage::flag_hidden) attr += 'h';
-				if (e.executable_attribute) attr += 'x';
+				if (flags & file_storage::flag_executable) attr += 'x';
-				if (m_include_symlinks && e.symlink_attribute) attr += 'l';
+				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));
 			}

src/disk_io_thread.cpp

@@ -835,7 +835,6 @@ namespace libtorrent
 		if (storage)
 		{
 			boost::unordered_set<cached_piece_entry*> const& pieces = storage->cached_pieces();
-			// TODO: 2 should this be allocated on the stack?
 			std::vector<int> piece_index;
 			piece_index.reserve(pieces.size());
 			for (boost::unordered_set<cached_piece_entry*>::const_iterator i = pieces.begin()

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
+	void file_storage::add_file(std::wstring const& file, boost::int64_t file_size
-		, std::time_t mtime, std::string const& symlink_path)
+		, 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
+	void file_storage::add_file(std::string const& path, boost::int64_t file_size
-		, std::time_t mtime, std::string const& symlink_path)
+		, int file_flags, std::time_t mtime, std::string const& symlink_path)
 	{
-		TORRENT_ASSERT_PRECOND(!is_complete(file));
+		add_file_borrow(NULL, 0, path, file_size, file_flags, NULL, mtime
-		TORRENT_ASSERT_PRECOND(size >= 0);
+			, symlink_path);
-		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;
 	}
 
-	// TODO: 2 it would be nice if file_entry::filehash could be taken into
+	void file_storage::add_file_borrow(char const* filename, int filename_len
-	// account as well, and if the file_storage object could actually hold
+		, std::string const& path, boost::int64_t file_size
-	// copies of filehash
+		, boost::uint32_t file_flags, char const* filehash
-	void file_storage::add_file(file_entry const& ent, char const* filehash)
+		, boost::int64_t mtime, std::string const& symlink_path)
 	{
-		TORRENT_ASSERT_PRECOND(ent.size >= 0);
+		TORRENT_ASSERT_PRECOND(file_size >= 0);
-		if (!has_parent_path(ent.path))
+		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);
+		internal_file_entry ife;
-		int file_index = m_files.size();
 		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

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
+	// top level is extracting the file for a single-file torrent. The
-		, std::string const& root_dir, lazy_entry const** filehash
+	// distinction is that the filename is found in "name" rather than
-		, lazy_entry const** filename, time_t* mtime)
+	// "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");
+		boost::int64_t file_size = dict.dict_find_int_value("length", -1);
-		if (length == 0 || length->type() != lazy_entry::int_t)
+		if (file_size < 0)
-			return false;
+		{
-		target.size = length->int_value();
+			ec = errors::torrent_invalid_length;
-		if (target.size < 0)
 			return false;
+		}
 
-		boost::int64_t ts = dict.dict_find_int_value("mtime", -1);
+		boost::int64_t mtime = dict.dict_find_int_value("mtime", 0);
-		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;
 
+		// 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;
+			file_flags = file_storage::flag_pad_file;
-
-		target.path = path;
 
 		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 'l': file_flags |= file_storage::flag_symlink; file_size = 0; break;
-					case 'x': target.executable_attribute = true; break;
+					case 'x': file_flags |= file_storage::flag_executable; break;
-					case 'h': target.hidden_attribute = true; break;
+					case 'h': file_flags |= file_storage::flag_hidden; break;
-					case 'p': target.pad_file = true; 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)
+		char const* filehash = NULL;
-			*filehash = fh;
+		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;
+			if (!extract_single_file(*list.list_at(i), target, root_dir
-			time_t mtime = 0;
+				, info_ptr_diff, false, ec))
-			file_entry e;
-			lazy_entry const* fee = 0;
-			if (!extract_single_file(*list.list_at(i), e, root_dir
-				, &file_hash, &fee, &mtime))
 				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;
+			if (!extract_single_file(info, files, "", info_ptr_diff, true, ec))
-			e.path = name;
-			e.offset = 0;
-			e.size = info.dict_find_int_value("length", -1);
-			if (e.size < 0)
-			{
-				ec = errors::torrent_invalid_length;
 				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))
+			if (!extract_files(*i, files, name, info_ptr_diff, ec))
-			{
-				ec = errors::torrent_file_parse_failed;
 				return false;
-			}
 			m_multifile = true;
 		}
 		TORRENT_ASSERT(!files.name().empty());

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

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<torrent_info> b(boost::make_shared<torrent_info>(*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<torrent_info> b(boost::make_shared<torrent_info>(*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());

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 },
+	{ "negative_file_size.torrent", errors::torrent_invalid_length },
-	{ "invalid_path_list.torrent", errors::torrent_file_parse_failed },
+	{ "invalid_path_list.torrent", errors::torrent_missing_name},
-	{ "missing_path_list.torrent", errors::torrent_file_parse_failed },
+	{ "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_root_hash2.torrent", errors::torrent_missing_pieces },
-	{ "invalid_file_size.torrent", errors::torrent_file_parse_failed },
+	{ "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->files().file_path(0) == 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(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->files().file_flags(0) & file_storage::flag_pad_file, false);
-			TEST_EQUAL(ti->file_at(1).pad_file, true);
+			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();

test/web_seed_suite.cpp

@@ -155,8 +155,8 @@ void test_transfer(lt::session& ses, boost::shared_ptr<torrent_info> torrent_fil
 	int pad_file_size = 0;
 	for (int i = 0; i < fs.num_files(); ++i)
 	{
-		file_entry f = fs.at(i);
+		if (fs.file_flags(i) & file_storage::flag_pad_file)
-		if (f.pad_file) pad_file_size += f.size;
+			pad_file_size += fs.file_size(i);
 	}
 
 	peer_disconnects = 0;