fix issue in internal_file_entry when assigning an empty string that's borrowed

include/libtorrent/file_storage.hpp

@@ -119,11 +119,11 @@ namespace libtorrent
 		friend class torrent_info;
 #endif
 		internal_file_entry()
-			: name(0)
+			: name(NULL)
 			, offset(0)
-			, symlink_index(-1)
+			, symlink_index(not_a_symlink)
 			, size(0)
-			, name_len(0)
+			, name_len(name_is_owned)
 			, pad_file(false)
 			, hidden_attribute(false)
 			, executable_attribute(false)
@@ -132,11 +132,11 @@ namespace libtorrent
 		{}
 
 		internal_file_entry(file_entry const& e)
-			: name(0)
+			: name(NULL)
 			, offset(e.offset)
-			, symlink_index(-1)
+			, symlink_index(not_a_symlink)
 			, size(e.size)
-			, name_len(0)
+			, name_len(name_is_owned)
 			, pad_file(e.pad_file)
 			, hidden_attribute(e.hidden_attribute)
 			, executable_attribute(e.executable_attribute)
@@ -151,7 +151,7 @@ namespace libtorrent
 
 		~internal_file_entry();
 
-		void set_name(char const* n, int borrow_chars = 0);
+		void set_name(char const* n, bool borrow_string = false, int string_len = 0);
 		std::string filename() const;
 
 		// make it available for logging
@@ -165,26 +165,30 @@ namespace libtorrent
 		char const* name;
 	public:
 
+		enum {
+			name_is_owned = (1<<12)-1,
+			not_a_symlink = (1<<16)-1
+		};
 		// the offset of this file inside the torrent
-		size_type offset:48;
+		boost::uint64_t offset:48;
 
-		// index into file_storage::m_symlinks or -1
+		// index into file_storage::m_symlinks or not_a_symlink
 		// if this is not a symlink
-		size_type symlink_index:16;
+		boost::uint64_t symlink_index:16;
 
 		// the size of this file
-		size_type size:48;
+		boost::uint64_t size:48;
 
 		// the number of characters in the name. If this is
-		// 0, name is null terminated and owned by this object
+		// name_is_owned, name is null terminated and owned by this object
 		// (i.e. it should be freed in the destructor). If
-		// the len is > 0, the name pointer doesn not belong
+		// the len is not name_is_owned, the name pointer doesn not belong
 		// to this object, and it's not null terminated
-		size_type name_len:10;
+		boost::uint64_t name_len:12;
-		bool pad_file:1;
+		boost::uint64_t pad_file:1;
-		bool hidden_attribute:1;
+		boost::uint64_t hidden_attribute:1;
-		bool executable_attribute:1;
+		boost::uint64_t executable_attribute:1;
-		bool symlink_attribute:1;
+		boost::uint64_t symlink_attribute: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

src/file_storage.cpp

@@ -111,7 +111,7 @@ namespace libtorrent
 
 	file_entry::~file_entry() {}
 
-	internal_file_entry::~internal_file_entry() { if (name_len == 0) 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)
@@ -142,26 +142,40 @@ namespace libtorrent
 		return *this;
 	}
 
-	void internal_file_entry::set_name(char const* n, int borrow_chars)
+	// if borrow_chars >= 0, don't take ownership over n, just
+	// point to it. It points to borrow_chars number of characters.
+	// if borrow_chars == -1, n is a null terminated string that
+	// should be copied 
+	void internal_file_entry::set_name(char const* n, bool borrow_string, int string_len)
 	{
-		TORRENT_ASSERT(borrow_chars >= 0);
+		TORRENT_ASSERT(string_len >= 0);
-		if (borrow_chars > 1023) borrow_chars = 1023;
+
-		if (name_len == 0) free((void*)name);
+		// we have limited space in the length field. truncate string
-		if (n == 0 || *n == 0)
+		// if it's too long
+		if (string_len >= name_is_owned) string_len = name_is_owned - 1;
+
+		// free the current string, before assigning the new one
+		if (name_len == name_is_owned) free((void*)name);
+		if (n == NULL)
 		{
-			TORRENT_ASSERT(borrow_chars == 0);
+			TORRENT_ASSERT(borrow_string == false);
-			name = 0;
+			name = NULL;
+		}
+		else if (borrow_string)
+		{
+			name = n;
+			name_len = string_len;
 		}
 		else
 		{
-			name = borrow_chars ? n : allocate_string_copy(n);
+			name = allocate_string_copy(n);
+			name_len = name_is_owned;
 		}
-		name_len = borrow_chars;
 	}
 
 	std::string internal_file_entry::filename() const
 	{
-		if (name_len) return std::string(name, name_len);
+		if (name_len != name_is_owned) return std::string(name, name_len);
 		return name ? name : "";
 	}
 
@@ -203,7 +217,7 @@ namespace libtorrent
 	void file_storage::rename_file_borrow(int index, char const* new_filename, int len)
 	{
 		TORRENT_ASSERT(index >= 0 && index < int(m_files.size()));
-		m_files[index].set_name(new_filename, len);
+		m_files[index].set_name(new_filename, true, len);
 	}
 
 	namespace
@@ -253,6 +267,8 @@ namespace libtorrent
 
 	int file_storage::file_name_len(int index) const
 	{
+		if (m_files[index].name_len == internal_file_entry::name_is_owned)
+			return -1;
 		return m_files[index].name_len;
 	}
 
@@ -285,7 +301,7 @@ namespace libtorrent
 				file_slice f;
 				f.file_index = file_iter - m_files.begin();
 				f.offset = file_offset + file_base(f.file_index);
-				f.size = (std::min)(file_iter->size - file_offset, (size_type)size);
+				f.size = (std::min)(size_type(file_iter->size) - file_offset, (size_type)size);
 				TORRENT_ASSERT(f.size <= size);
 				size -= int(f.size);
 				file_offset += f.size;
@@ -311,7 +327,7 @@ namespace libtorrent
 		ret.hidden_attribute = ife.hidden_attribute;
 		ret.executable_attribute = ife.executable_attribute;
 		ret.symlink_attribute = ife.symlink_attribute;
-		if (ife.symlink_index >= 0) 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;
 	}
@@ -415,7 +431,6 @@ namespace libtorrent
 		int file_index = m_files.size();
 		m_files.push_back(ife);
 		internal_file_entry& e = m_files.back();
-		if (e.size < 0) e.size = 0;
 		e.offset = m_total_size;
 		m_total_size += e.size;
 		if (filehash)

src/torrent_info.cpp

@@ -1399,7 +1399,7 @@ namespace libtorrent
 		for (int i = 0; i < m_files.num_files(); ++i)
 		{
 			TORRENT_ASSERT(m_files.file_name_ptr(i) != 0);
-			if (m_files.file_name_len(i) > 0)
+			if (m_files.file_name_len(i) != -1)
 			{
 				// name needs to point into the allocated info section buffer
 				TORRENT_ASSERT(m_files.file_name_ptr(i) >= m_info_section.get());

test/test_primitives.cpp

@@ -83,7 +83,6 @@ int test_main()
 	using namespace libtorrent;
 	using namespace libtorrent::dht;
 	error_code ec;
-	int ret = 0;
 
 	// make sure the retry interval keeps growing
 	// on failing announces