diff --git a/include/libtorrent/file_storage.hpp b/include/libtorrent/file_storage.hpp index 86c860a0a..069397c26 100644 --- a/include/libtorrent/file_storage.hpp +++ b/include/libtorrent/file_storage.hpp @@ -530,6 +530,11 @@ namespace libtorrent // to file at ``index``. int file_flags(int index) const; + // returns true if the file at the specified index has been renamed to + // have an absolute path, i.e. is not anchored in the save path of the + // torrent. + bool file_absolute_path(int index) const; + // returns the index of the file at the given offset in the torrent int file_index_at_offset(boost::int64_t offset) const; diff --git a/src/file_storage.cpp b/src/file_storage.cpp index db11be4f0..41fde9ac6 100644 --- a/src/file_storage.cpp +++ b/src/file_storage.cpp @@ -816,6 +816,12 @@ namespace libtorrent | (fe.symlink_attribute ? flag_symlink : 0); } + bool file_storage::file_absolute_path(int index) const + { + internal_file_entry const& fe = m_files[index]; + return fe.path_index == -2; + } + #ifndef TORRENT_NO_DEPRECATE void file_storage::set_file_base(int index, boost::int64_t off) { diff --git a/src/storage.cpp b/src/storage.cpp index 2abea7490..18cd4f641 100644 --- a/src/storage.cpp +++ b/src/storage.cpp @@ -795,8 +795,8 @@ namespace libtorrent typedef std::set::iterator iter_t; for (int i = 0; i < files().num_files(); ++i) { - std::string fp = files().file_path(i); - bool complete = is_complete(fp); + std::string const fp = files().file_path(i); + bool const complete = files().file_absolute_path(i); std::string p = complete ? fp : combine_path(m_save_path, fp); if (!complete) { @@ -1124,32 +1124,32 @@ namespace libtorrent return true; } - int default_storage::move_storage(std::string const& sp, int flags, storage_error& ec) + int default_storage::move_storage(std::string const& sp, int const flags + , storage_error& ec) { int ret = piece_manager::no_error; - std::string save_path = complete(sp); + std::string const save_path = complete(sp); // check to see if any of the files exist - error_code e; file_storage const& f = files(); - file_status s; if (flags == fail_if_exist) { - stat_file(save_path, &s, e); - if (e != boost::system::errc::no_such_file_or_directory) + file_status s; + error_code err; + stat_file(save_path, &s, err); + if (err != boost::system::errc::no_such_file_or_directory) { // the directory exists, check all the files for (int i = 0; i < f.num_files(); ++i) { // files moved out to absolute paths are ignored - if (is_complete(f.file_path(i))) continue; + if (f.file_absolute_path(i)) continue; - std::string new_path = f.file_path(i, save_path); - stat_file(new_path, &s, e); - if (e != boost::system::errc::no_such_file_or_directory) + stat_file(f.file_path(i, save_path), &s, err); + if (err != boost::system::errc::no_such_file_or_directory) { - ec.ec = e; + ec.ec = err; ec.file = i; ec.operation = storage_error::stat; return piece_manager::file_exist; @@ -1158,26 +1158,30 @@ namespace libtorrent } } - e.clear(); - stat_file(save_path, &s, e); - if (e == boost::system::errc::no_such_file_or_directory) { - create_directories(save_path, e); - if (e) + file_status s; + error_code err; + stat_file(save_path, &s, err); + if (err == boost::system::errc::no_such_file_or_directory) { - ec.ec = e; + err.clear(); + create_directories(save_path, err); + if (err) + { + ec.ec = err; + ec.file = -1; + ec.operation = storage_error::mkdir; + return piece_manager::fatal_disk_error; + } + } + else if (err) + { + ec.ec = err; ec.file = -1; - ec.operation = storage_error::mkdir; + ec.operation = storage_error::stat; return piece_manager::fatal_disk_error; } } - else if (e) - { - ec.ec = e; - ec.file = -1; - ec.operation = storage_error::mkdir; - return piece_manager::fatal_disk_error; - } m_pool.release(this); @@ -1186,13 +1190,14 @@ namespace libtorrent #endif int i; + error_code e; for (i = 0; i < f.num_files(); ++i) { // files moved out to absolute paths are not moved - if (is_complete(f.file_path(i))) continue; + if (f.file_absolute_path(i)) continue; - std::string old_path = combine_path(m_save_path, f.file_path(i)); - std::string new_path = combine_path(save_path, f.file_path(i)); + 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 (flags == dont_replace && exists(new_path)) { @@ -1200,7 +1205,9 @@ namespace libtorrent continue; } - e.clear(); + // TODO: ideally, if we end up copying files because of a move across + // 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 @@ -1233,10 +1240,10 @@ namespace libtorrent while (--i >= 0) { // files moved out to absolute paths are not moved - if (is_complete(f.file_path(i))) continue; + if (f.file_absolute_path(i)) continue; - std::string old_path = combine_path(m_save_path, f.file_path(i)); - std::string new_path = combine_path(save_path, f.file_path(i)); + 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)) { @@ -1249,20 +1256,21 @@ namespace libtorrent return piece_manager::fatal_disk_error; } - m_save_path.swap(save_path); - // now we have old save path in save_path + std::string const old_save_path = m_save_path; + m_save_path = save_path; std::set subdirs; for (i = 0; i < f.num_files(); ++i) { // files moved out to absolute paths are not moved - if (is_complete(f.file_path(i))) continue; + if (f.file_absolute_path(i)) continue; if (has_parent_path(f.file_path(i))) subdirs.insert(parent_path(f.file_path(i))); - std::string old_path = combine_path(save_path, f.file_path(i)); - // we may still have some files in old save_path + 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 // eg. if (flags == dont_replace && exists(new_path)) // ignore errors when removing error_code ignore; @@ -1272,11 +1280,11 @@ namespace libtorrent for (std::set::iterator it(subdirs.begin()) , end(subdirs.end()); it != end; ++it) { - e.clear(); - std::string subdir(*it); - while (!subdir.empty() && !e) + error_code err; + std::string subdir = combine_path(old_save_path, *it); + while (subdir != old_save_path && !err) { - remove(combine_path(save_path, subdir), e); + remove(subdir, err); subdir = parent_path(subdir); } } diff --git a/test/test_storage.cpp b/test/test_storage.cpp index 519996b9a..fef4a36e4 100644 --- a/test/test_storage.cpp +++ b/test/test_storage.cpp @@ -1251,15 +1251,22 @@ TORRENT_TEST(readwritev_zero_size_files) TEST_CHECK(check_pattern(buf, 0)); } -TORRENT_TEST(move_storage_into_self) +void delete_dirs(std::string path) { error_code ec; - std::string save_path = current_working_directory(); - remove_all(combine_path(save_path, "temp_storage"), ec); + remove_all(path, ec); if (ec && ec != boost::system::errc::no_such_file_or_directory) - std::cerr << "remove_all '" << combine_path(save_path, "temp_storage") - << "': " << ec.message() << std::endl; - TEST_CHECK(!exists(combine_path(save_path, "temp_storage"))); + { + fprintf(stderr, "remove_all \"%s\": %s\n" + , path.c_str(), ec.message().c_str()); + } + TEST_CHECK(!exists(path)); +} + +TORRENT_TEST(move_storage_into_self) +{ + std::string const save_path = current_working_directory(); + delete_dirs(combine_path(save_path, "temp_storage")); aux::session_settings set; file_storage fs; @@ -1269,11 +1276,11 @@ TORRENT_TEST(move_storage_into_self) 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 b = {&buf[0], 4}; + file::iovec_t const b = {&buf[0], 4}; storage_error se; s->writev(&b, 1, 2, 0, 0, se); - std::string test_path = combine_path(save_path, combine_path("temp_storage", "folder1")); + std::string const test_path = combine_path(save_path, combine_path("temp_storage", "folder1")); s->move_storage(test_path, 0, se); TEST_EQUAL(se.ec, boost::system::errc::success); @@ -1289,21 +1296,11 @@ TORRENT_TEST(move_storage_into_self) TORRENT_TEST(dont_move_intermingled_files) { - error_code ec; - - std::string save_path = combine_path(current_working_directory(), "save_path_1"); - remove_all(combine_path(save_path, "temp_storage"), ec); - if (ec && ec != boost::system::errc::no_such_file_or_directory) - std::cerr << "remove_all '" << combine_path(save_path, "temp_storage") - << "': " << ec.message() << std::endl; - TEST_CHECK(!exists(combine_path(save_path, "temp_storage"))); + std::string const save_path = combine_path(current_working_directory(), "save_path_1"); + delete_dirs(combine_path(save_path, "temp_storage")); std::string test_path = combine_path(current_working_directory(), "save_path_2"); - remove_all(combine_path(test_path, "temp_storage"), ec); - if (ec && ec != boost::system::errc::no_such_file_or_directory) - std::cerr << "remove_all '" << combine_path(test_path, "temp_storage") - << "': " << ec.message() << std::endl; - TEST_CHECK(!exists(combine_path(test_path, "temp_storage"))); + delete_dirs(combine_path(test_path, "temp_storage")); aux::session_settings set; file_storage fs; @@ -1317,6 +1314,7 @@ TORRENT_TEST(dont_move_intermingled_files) storage_error se; s->writev(&b, 1, 2, 0, 0, se); + error_code ec; create_directory(combine_path(save_path, combine_path("temp_storage" , combine_path("_folder3", "alien_folder1"))), ec); TEST_EQUAL(ec, boost::system::errc::success);