fix move_storage() to its own directory (would delete the files)

ChangeLog

@@ -1,3 +1,4 @@
+	* fix move_storage() to its own directory (would delete the files)
 	* fix socks5 support for UDP
 	* add setting urlseed_max_request_bytes to handle large web seed requests
 	* fix python build with CC/CXX environment

src/file.cpp

@@ -702,27 +702,6 @@ namespace libtorrent
 		}
 
 		rename(inf, newf, ec);
-
-		// on OSX, the error when trying to rename a file across different
-		// volumes is EXDEV, which will make it fall back to copying.
-
-		if (ec)
-		{
-			if (ec != boost::system::errc::no_such_file_or_directory
-				&& ec != boost::system::errc::invalid_argument
-				&& ec != boost::system::errc::permission_denied)
-			{
-				ec.clear();
-				copy_file(inf, newf, ec);
-
-				if (!ec)
-				{
-					// ignore errors when removing
-					error_code ignore;
-					remove(inf, ignore);
-				}
-			}
-		}
 	}
 
 	std::string split_path(std::string const& f)

src/storage.cpp

@@ -1216,6 +1216,10 @@ namespace libtorrent
 		print_open_files("release files", m_files.name().c_str());
 #endif
 
+		// indices of all files we ended up copying. These need to be deleted
+		// later
+		std::vector<bool> copied_files(f.num_files(), false);
+
 		int i;
 		error_code e;
 		for (i = 0; i < f.num_files(); ++i)
@@ -1236,10 +1240,22 @@ namespace libtorrent
 			// volumes, the source should not be deleted until they've all been
 			// copied. That would let us rollback with higher confidence.
 			move_file(old_path, new_path, e);
+
 			// if the source file doesn't exist. That's not a problem
 			// we just ignore that file
 			if (e == boost::system::errc::no_such_file_or_directory)
 				e.clear();
+			else if (e
+				&& e != boost::system::errc::invalid_argument
+				&& e != boost::system::errc::permission_denied)
+			{
+				// moving the file failed
+				// on OSX, the error when trying to rename a file across different
+				// volumes is EXDEV, which will make it fall back to copying.
+				e.clear();
+				copy_file(old_path, new_path, e);
+				if (!e) copied_files[i] = true;
+			}
 
 			if (e)
 			{
@@ -1269,15 +1285,16 @@ namespace libtorrent
 				// files moved out to absolute paths are not moved
 				if (f.file_absolute_path(i)) continue;
 
+				// if we ended up copying the file, don't do anything during
+				// roll-back
+				if (copied_files[i]) continue;
+
 				std::string const old_path = combine_path(m_save_path, f.file_path(i));
 				std::string const new_path = combine_path(save_path, f.file_path(i));
 
-				if (!exists(old_path))
-				{
-					// ignore errors when rolling back
-					error_code ignore;
-					move_file(new_path, old_path, ignore);
-				}
+				// ignore errors when rolling back
+				error_code ignore;
+				move_file(new_path, old_path, ignore);
 			}
 
 			return piece_manager::fatal_disk_error;
@@ -1295,6 +1312,10 @@ namespace libtorrent
 			if (has_parent_path(f.file_path(i)))
 				subdirs.insert(parent_path(f.file_path(i)));
 
+			// if we ended up renaming the file instead of moving it, there's no
+			// need to delete the source.
+			if (copied_files[i] == false) continue;
+
 			std::string const old_path = combine_path(old_save_path, f.file_path(i));
 
 			// we may still have some files in old old_save_path
@@ -1305,7 +1326,7 @@ namespace libtorrent
 		}
 
 		for (std::set<std::string>::iterator it(subdirs.begin())
-			 , end(subdirs.end()); it != end; ++it)
+			, end(subdirs.end()); it != end; ++it)
 		{
 			error_code err;
 			std::string subdir = combine_path(old_save_path, *it);

test/test_storage.cpp

@@ -1333,6 +1333,37 @@ void delete_dirs(std::string path)
 	TEST_CHECK(!exists(path));
 }
 
+TORRENT_TEST(move_storage_to_self)
+{
+	// call move_storage with the path to the exising storage. should be a no-op
+	std::string const save_path = current_working_directory();
+	std::string const test_path = combine_path(save_path, "temp_storage");
+	delete_dirs(test_path);
+
+	aux::session_settings set;
+	file_storage fs;
+	std::vector<char> buf;
+	file_pool fp;
+	io_service ios;
+	disk_buffer_pool dp(16 * 1024, ios, boost::bind(&nop));
+	boost::shared_ptr<default_storage> s = setup_torrent(fs, fp, buf, save_path, set);
+
+	file::iovec_t const b = {&buf[0], 4};
+	storage_error se;
+	s->writev(&b, 1, 2, 0, 0, se);
+
+	TEST_CHECK(exists(combine_path(test_path, combine_path("folder2", "test3.tmp"))));
+	TEST_CHECK(exists(combine_path(test_path, combine_path("_folder3", "test4.tmp"))));
+
+	s->move_storage(save_path, 0, se);
+	TEST_EQUAL(se.ec, boost::system::errc::success);
+
+	TEST_CHECK(exists(test_path));
+
+	TEST_CHECK(exists(combine_path(test_path, combine_path("folder2", "test3.tmp"))));
+	TEST_CHECK(exists(combine_path(test_path, combine_path("_folder3", "test4.tmp"))));
+}
+
 TORRENT_TEST(move_storage_into_self)
 {
 	std::string const save_path = current_working_directory();