From acf77a53855d848c1b6241dde298a69f3b2dd254 Mon Sep 17 00:00:00 2001
From: Arvid Norberg <arvid.norberg@gmail.com>
Date: Sun, 18 Sep 2016 20:08:27 -0400
Subject: [PATCH] fix tail-padding for last file in create_torrent (#1106)

---
 ChangeLog                  |  1 +
 src/file_storage.cpp       | 14 +++++----
 test/test_file_storage.cpp | 61 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a58912422..2899f7cf6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,4 @@
+	* fix tail-padding for last file in create_torrent
 	* fix internal resolve links lookup for mutable torrents
 	* hint DHT bootstrap nodes of actual bootstrap request
 
diff --git a/src/file_storage.cpp b/src/file_storage.cpp
index 834a075f1..e08eb9cea 100644
--- a/src/file_storage.cpp
+++ b/src/file_storage.cpp
@@ -182,7 +182,7 @@ namespace libtorrent
 			&& std::memcmp(branch_path, m_name.c_str(), m_name.size()) == 0)
 		{
 			// the +1 is to skip the trailing '/' (or '\')
-			int offset = m_name.size()
+			int const offset = m_name.size()
 				+ (m_name.size() == branch_len?0:1);
 			branch_path += offset;
 			branch_len -= offset;
@@ -1041,11 +1041,13 @@ namespace libtorrent
 				++i;
 
 				// tail-padding is enabled, and the offset after this file is not
-				// aligned and it's not the last file. The last file must be padded
-				// too, in order to match an equivalent tail-padded file.
+				// aligned. The last file must be padded too, in order to match an
+				// equivalent tail-padded file.
 				add_pad_file(alignment - (off % alignment), i, off, padding_file);
 
 				TORRENT_ASSERT((off % alignment) == 0);
+
+				if (i == m_files.end()) break;
 			}
 		}
 		m_total_size = off;
@@ -1056,8 +1058,8 @@ namespace libtorrent
 		, boost::int64_t& offset
 		, int& pad_file_counter)
 	{
-		int cur_index = i - m_files.begin();
-		int index = m_files.size();
+		int const cur_index = i - m_files.begin();
+		int const index = m_files.size();
 		m_files.push_back(internal_file_entry());
 		++m_num_files;
 		internal_file_entry& e = m_files.back();
@@ -1080,7 +1082,7 @@ namespace libtorrent
 		if (!m_file_base.empty()) m_file_base.resize(index + 1, 0);
 #endif
 
-		reorder_file(index, cur_index);
+		if (index != cur_index) reorder_file(index, cur_index);
 	}
 
 	void file_storage::unload()
diff --git a/test/test_file_storage.cpp b/test/test_file_storage.cpp
index ccb4b0ac7..dd4b3fa55 100644
--- a/test/test_file_storage.cpp
+++ b/test/test_file_storage.cpp
@@ -211,7 +211,66 @@ TORRENT_TEST(file_path_hash)
 	TEST_EQUAL(file_hash0, file_hash1);
 }
 
-// TODO: test file_storage::optimize
+// make sure we pad the end of the torrent when tail_padding is specified
+TORRENT_TEST(optimize_tail_padding)
+{
+	file_storage fs;
+	fs.set_piece_length(512);
+	fs.add_file(combine_path("s", "1"), 700);
+
+	fs.optimize(512, 512, true);
+
+	// since the size of file 3 is a multiple of the alignment (512), it should
+	// be prioritized, to minimize the amount of padding.
+	// after that, we want to pick the largest file (2), and since file 1 is
+	// smaller than the pad-file limit (512) we won't pad it. Since tail_padding
+	// is false, we won't pad the tail of the torrent either
+
+	TEST_EQUAL(fs.num_files(), 2);
+
+	TEST_EQUAL(fs.file_size(0), 700);
+	TEST_EQUAL(fs.file_name(0), "1");
+	TEST_EQUAL(fs.pad_file_at(0), false);
+
+	TEST_EQUAL(fs.file_size(1), 1024 - 700);
+	TEST_EQUAL(fs.pad_file_at(1), true);
+}
+
+
+// make sure we fill in padding with small files
+TORRENT_TEST(optimize_pad_fillers)
+{
+	file_storage fs;
+	fs.set_piece_length(512);
+	fs.add_file(combine_path("s", "1"), 1);
+	fs.add_file(combine_path("s", "2"), 1000);
+	fs.add_file(combine_path("s", "3"), 1001);
+
+	fs.optimize(512, 512, false);
+
+	// first we pick the largest file, then we need to add padding, since file 1
+	// is smaller than the pad file limit, it won't be aligned anyway, so we
+	// place that as part of the padding
+
+	TEST_EQUAL(fs.num_files(), 4);
+
+	TEST_EQUAL(fs.file_size(0), 1001);
+	TEST_EQUAL(fs.file_name(0), "3");
+	TEST_EQUAL(fs.pad_file_at(0), false);
+
+	TEST_EQUAL(fs.file_size(1), 1);
+	TEST_EQUAL(fs.file_name(1), "1");
+	TEST_EQUAL(fs.pad_file_at(1), false);
+
+	TEST_EQUAL(fs.file_size(2), 1024 - (1001 + 1));
+	TEST_EQUAL(fs.pad_file_at(2), true);
+
+	TEST_EQUAL(fs.file_size(3), 1000);
+	TEST_EQUAL(fs.file_name(3), "2");
+	TEST_EQUAL(fs.pad_file_at(3), false);
+}
+
+// TODO: add more optimize() tests
 // TODO: test map_block
 // TODO: test piece_size(int piece)
 // TODO: test file_index_at_offset