From 7957e2a30ecca4da4ac3cfa6cca938467f2a65b9 Mon Sep 17 00:00:00 2001 From: arvidn Date: Sun, 1 Jan 2017 18:04:18 -0500 Subject: [PATCH] fix move_storage() to its own directory (would delete the files) --- ChangeLog | 1 + src/file.cpp | 21 --------------------- src/storage.cpp | 35 ++++++++++++++++++++++++++++------- test/test_storage.cpp | 31 +++++++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 28 deletions(-) diff --git a/ChangeLog b/ChangeLog index 995f2d049..b5102be47 100644 --- a/ChangeLog +++ b/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 diff --git a/src/file.cpp b/src/file.cpp index 13bffc180..7aee4bdf7 100644 --- a/src/file.cpp +++ b/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) diff --git a/src/storage.cpp b/src/storage.cpp index 5ea984c19..43cddf09e 100644 --- a/src/storage.cpp +++ b/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 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::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); diff --git a/test/test_storage.cpp b/test/test_storage.cpp index edd5c1bcb..73a14020e 100644 --- a/test/test_storage.cpp +++ b/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 buf; + file_pool fp; + io_service ios; + disk_buffer_pool dp(16 * 1024, ios, boost::bind(&nop)); + boost::shared_ptr 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();