From b27238a48041256d08db3e8cc8dcc8a9dda38348 Mon Sep 17 00:00:00 2001 From: David Sansome Date: Tue, 10 Aug 2010 19:42:43 +0000 Subject: [PATCH] Test whether we can still talk to afc before freeing the lockdownd client (fixes usbmuxd errors once and for all), report afc copy errors, keep track of files that failed to copy to a device, don't attempt to write the itunesdb if there were errors during copying. --- src/core/deletefiles.cpp | 8 +++-- src/core/deletefiles.h | 2 ++ src/core/musicstorage.h | 4 +-- src/core/organise.cpp | 8 +++-- src/core/organise.h | 2 ++ src/core/utilities.cpp | 41 ++++++++++++++++++++++ src/core/utilities.h | 3 ++ src/devices/afcdevice.cpp | 11 +++--- src/devices/afcdevice.h | 2 +- src/devices/afctransfer.cpp | 45 ++----------------------- src/devices/afctransfer.h | 2 -- src/devices/gpoddevice.cpp | 36 ++++++++++---------- src/devices/gpoddevice.h | 4 +-- src/devices/imobiledeviceconnection.cpp | 10 ++++++ 14 files changed, 100 insertions(+), 78 deletions(-) diff --git a/src/core/deletefiles.cpp b/src/core/deletefiles.cpp index 59a558800..cbd4ac63f 100644 --- a/src/core/deletefiles.cpp +++ b/src/core/deletefiles.cpp @@ -76,7 +76,7 @@ void DeleteFiles::ProcessSomeFiles() { if (progress_ >= songs_.count()) { task_manager_->SetTaskProgress(task_id_, progress_, songs_.count()); - storage_->FinishCopy(); + storage_->FinishCopy(songs_with_errors_.isEmpty()); task_manager_->SetTaskFinished(task_id_); @@ -96,7 +96,11 @@ void DeleteFiles::ProcessSomeFiles() { for ( ; progress_SetTaskProgress(task_id_, progress_, songs_.count()); - storage_->DeleteFromStorage(songs_.at(progress_)); + const Song& song = songs_[progress_]; + + if (!storage_->DeleteFromStorage(song)) { + songs_with_errors_ << song; + } } QTimer::singleShot(0, this, SLOT(ProcessSomeFiles())); diff --git a/src/core/deletefiles.h b/src/core/deletefiles.h index b07b17f87..70a022a9d 100644 --- a/src/core/deletefiles.h +++ b/src/core/deletefiles.h @@ -53,6 +53,8 @@ private: int task_id_; int progress_; + + SongList songs_with_errors_; }; #endif // DELETEFILES_H diff --git a/src/core/musicstorage.h b/src/core/musicstorage.h index 1fe9bf0a7..32b254e94 100644 --- a/src/core/musicstorage.h +++ b/src/core/musicstorage.h @@ -40,11 +40,11 @@ public: virtual bool CopyToStorage(const QString& source, const QString& destination, const Song& metadata, bool overwrite, bool remove_original) = 0; - virtual void FinishCopy() {} + virtual void FinishCopy(bool success) {} virtual void StartDelete() {} virtual bool DeleteFromStorage(const Song& metadata) = 0; - virtual void FinishDelete() {} + virtual void FinishDelete(bool success) {} virtual void Eject() {} }; diff --git a/src/core/organise.cpp b/src/core/organise.cpp index cfffff5ba..4ad5200f7 100644 --- a/src/core/organise.cpp +++ b/src/core/organise.cpp @@ -68,7 +68,7 @@ void Organise::ProcessSomeFiles() { if (progress_ >= files_.count()) { task_manager_->SetTaskProgress(task_id_, progress_, files_.count()); - destination_->FinishCopy(); + destination_->FinishCopy(files_with_errors_.isEmpty()); if (eject_after_) destination_->Eject(); @@ -110,8 +110,10 @@ void Organise::ProcessSomeFiles() { if (!song.is_valid()) continue; - destination_->CopyToStorage(filename, format_.GetFilenameForSong(song), - song, overwrite_, !copy_); + if (!destination_->CopyToStorage(filename, format_.GetFilenameForSong(song), + song, overwrite_, !copy_)) { + files_with_errors_ << filename; + } } QTimer::singleShot(0, this, SLOT(ProcessSomeFiles())); diff --git a/src/core/organise.h b/src/core/organise.h index 8f40726c4..58e548323 100644 --- a/src/core/organise.h +++ b/src/core/organise.h @@ -58,6 +58,8 @@ private: int task_id_; int progress_; + + QStringList files_with_errors_; }; #endif // ORGANISE_H diff --git a/src/core/utilities.cpp b/src/core/utilities.cpp index b34d8bece..60aa95ce8 100644 --- a/src/core/utilities.cpp +++ b/src/core/utilities.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -135,4 +136,44 @@ void RemoveRecursive(const QString& path) { dir.rmdir(path); } +bool Copy(QIODevice* source, QIODevice* destination) { + if (!source->open(QIODevice::ReadOnly)) + return false; + + if (!destination->open(QIODevice::WriteOnly)) + return false; + + const qint64 bytes = source->size(); + char* data = new char[bytes]; + qint64 pos = 0; + + forever { + const qint64 bytes_read = source->read(data + pos, bytes - pos); + if (bytes_read == -1) { + delete[] data; + return false; + } + + pos += bytes_read; + if (bytes_read == 0 || pos == bytes) + break; + } + + pos = 0; + forever { + const qint64 bytes_written = destination->write(data + pos, bytes - pos); + if (bytes_written == -1) { + delete[] data; + return false; + } + + pos += bytes_written; + if (bytes_written == 0 || pos == bytes) + break; + } + + delete[] data; + return true; +} + } // namespace diff --git a/src/core/utilities.h b/src/core/utilities.h index 726caf731..3e4712a47 100644 --- a/src/core/utilities.h +++ b/src/core/utilities.h @@ -19,6 +19,8 @@ #include +class QIODevice; + namespace Utilities { QString PrettyTime(int seconds); QString PrettySize(quint64 bytes); @@ -29,6 +31,7 @@ namespace Utilities { QString MakeTempDir(); void RemoveRecursive(const QString& path); + bool Copy(QIODevice* source, QIODevice* destination); } #endif // UTILITIES_H diff --git a/src/devices/afcdevice.cpp b/src/devices/afcdevice.cpp index 24dc73329..4823001e0 100644 --- a/src/devices/afcdevice.cpp +++ b/src/devices/afcdevice.cpp @@ -94,9 +94,8 @@ bool AfcDevice::CopyToStorage( { QFile source_file(source); AfcFile dest_file(&connection, dest); - source_file.open(QIODevice::ReadOnly); - dest_file.open(QIODevice::WriteOnly); - dest_file.write(source_file.readAll()); + if (!Utilities::Copy(&source_file, &dest_file)) + return false; } track->transferred = 1; @@ -111,12 +110,10 @@ bool AfcDevice::CopyToStorage( else track->filetype_marker |= suffix[i].toAscii(); } - qDebug() << track->filetype_marker; // Set the filename track->ipod_path = strdup(dest.toUtf8().constData()); itdb_filename_fs2ipod(track->ipod_path); - qDebug() << track->ipod_path; AddTrackToModel(track, "afc://" + url_.host()); @@ -128,11 +125,11 @@ bool AfcDevice::CopyToStorage( return true; } -void AfcDevice::FinishCopy() { +void AfcDevice::FinishCopy(bool success) { // Temporarily unset the GUID so libgpod doesn't lock the device for syncing itdb_device_set_sysinfo(db_->device, "FirewireGuid", NULL); - GPodDevice::FinishCopy(); + GPodDevice::FinishCopy(success); } void AfcDevice::FinaliseDatabase() { diff --git a/src/devices/afcdevice.h b/src/devices/afcdevice.h index fe6b63e3a..cbf6d4311 100644 --- a/src/devices/afcdevice.h +++ b/src/devices/afcdevice.h @@ -42,7 +42,7 @@ public: bool CopyToStorage(const QString &source, const QString &destination, const Song &metadata, bool overwrite, bool remove_original); - void FinishCopy(); + void FinishCopy(bool success); bool DeleteFromStorage(const Song &metadata); diff --git a/src/devices/afctransfer.cpp b/src/devices/afctransfer.cpp index 104e0d9da..f75bf5236 100644 --- a/src/devices/afctransfer.cpp +++ b/src/devices/afctransfer.cpp @@ -18,6 +18,7 @@ #include "afctransfer.h" #include "imobiledeviceconnection.h" #include "core/taskmanager.h" +#include "core/utilities.h" #include #include @@ -117,52 +118,12 @@ bool AfcTransfer::CopyFileFromDevice(iMobileDeviceConnection *c, const QString & QFile dest(local_filename); AfcFile source(c, path); - return Copy(&source, &dest); + return Utilities::Copy(&source, &dest); } bool AfcTransfer::CopyFileToDevice(iMobileDeviceConnection *c, const QString &path) { QFile source(local_destination_ + path); AfcFile dest(c, path); - return Copy(&source, &dest); -} - -bool AfcTransfer::Copy(QIODevice* source, QIODevice* destination) { - if (!source->open(QIODevice::ReadOnly)) - return false; - - if (!destination->open(QIODevice::WriteOnly)) - return false; - - const qint64 bytes = source->size(); - char* data = new char[bytes]; - qint64 pos = 0; - - forever { - const qint64 bytes_read = source->read(data + pos, bytes - pos); - if (bytes_read == -1) { - delete[] data; - return false; - } - - pos += bytes_read; - if (bytes_read == 0 || pos == bytes) - break; - } - - pos = 0; - forever { - const qint64 bytes_written = destination->write(data + pos, bytes - pos); - if (bytes_written == -1) { - delete[] data; - return false; - } - - pos += bytes_written; - if (bytes_written == 0 || pos == bytes) - break; - } - - delete[] data; - return true; + return Utilities::Copy(&source, &dest); } diff --git a/src/devices/afctransfer.h b/src/devices/afctransfer.h index f6671ee46..25a3ae454 100644 --- a/src/devices/afctransfer.h +++ b/src/devices/afctransfer.h @@ -51,8 +51,6 @@ private: bool CopyFileFromDevice(iMobileDeviceConnection* c, const QString& path); bool CopyFileToDevice(iMobileDeviceConnection* c, const QString& path); - static bool Copy(QIODevice* source, QIODevice* destination); - private: boost::shared_ptr device_; QThread* original_thread_; diff --git a/src/devices/gpoddevice.cpp b/src/devices/gpoddevice.cpp index f81c9e394..49ab396aa 100644 --- a/src/devices/gpoddevice.cpp +++ b/src/devices/gpoddevice.cpp @@ -129,22 +129,24 @@ bool GPodDevice::CopyToStorage( return true; } -void GPodDevice::FinishCopy() { - // Write the itunes database - GError* error = NULL; - itdb_write(db_, &error); - if (error) { - qDebug() << "GPodDevice error:" << error->message; - emit Error(QString::fromUtf8(error->message)); - g_error_free(error); - } else { - FinaliseDatabase(); +void GPodDevice::FinishCopy(bool success) { + if (success) { + // Write the itunes database + GError* error = NULL; + itdb_write(db_, &error); + if (error) { + qDebug() << "GPodDevice error:" << error->message; + emit Error(QString::fromUtf8(error->message)); + g_error_free(error); + } else { + FinaliseDatabase(); - // Update the library model - if (!songs_to_add_.isEmpty()) - backend_->AddOrUpdateSongs(songs_to_add_); - if (!songs_to_remove_.isEmpty()) - backend_->DeleteSongs(songs_to_remove_); + // Update the library model + if (!songs_to_add_.isEmpty()) + backend_->AddOrUpdateSongs(songs_to_add_); + if (!songs_to_remove_.isEmpty()) + backend_->DeleteSongs(songs_to_remove_); + } } songs_to_add_.clear(); @@ -197,7 +199,7 @@ bool GPodDevice::DeleteFromStorage(const Song& metadata) { return true; } -void GPodDevice::FinishDelete() { - FinishCopy(); +void GPodDevice::FinishDelete(bool success) { + FinishCopy(success); } diff --git a/src/devices/gpoddevice.h b/src/devices/gpoddevice.h index a9e243d77..ecbdf0130 100644 --- a/src/devices/gpoddevice.h +++ b/src/devices/gpoddevice.h @@ -44,11 +44,11 @@ public: void StartCopy(); bool CopyToStorage(const QString &source, const QString &destination, const Song &metadata, bool overwrite, bool remove_original); - void FinishCopy(); + void FinishCopy(bool success); void StartDelete(); bool DeleteFromStorage(const Song& metadata); - void FinishDelete(); + void FinishDelete(bool success); protected slots: void LoadFinished(Itdb_iTunesDB* db); diff --git a/src/devices/imobiledeviceconnection.cpp b/src/devices/imobiledeviceconnection.cpp index 22d885d4f..ca48efcb2 100644 --- a/src/devices/imobiledeviceconnection.cpp +++ b/src/devices/imobiledeviceconnection.cpp @@ -52,8 +52,18 @@ iMobileDeviceConnection::iMobileDeviceConnection(const QString& uuid) iMobileDeviceConnection::~iMobileDeviceConnection() { if (afc_) { + // Do a test to see if we can still talk to the device. If not, it's + // probably not safe to free the lockdownd client. + char* model = NULL; + afc_error_t err = afc_get_device_info_key(afc_, "Model", &model); + free(model); + + if (err != AFC_E_SUCCESS) + broken_ = true; + afc_client_free(afc_); } + if (lockdown_ && !broken_) { lockdownd_client_free(lockdown_); }