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

@@ -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_<n ; ++progress_) {
     task_manager_->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()));

src/core/deletefiles.h

@@ -53,6 +53,8 @@ private:
 
   int task_id_;
   int progress_;
+
+  SongList songs_with_errors_;
 };
 
 #endif // DELETEFILES_H

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() {}
 };

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),
+    if (!destination_->CopyToStorage(filename, format_.GetFilenameForSong(song),
-                                song, overwrite_, !copy_);
+                                     song, overwrite_, !copy_)) {
+      files_with_errors_ << filename;
+    }
   }
 
   QTimer::singleShot(0, this, SLOT(ProcessSomeFiles()));

src/core/organise.h

@@ -58,6 +58,8 @@ private:
 
   int task_id_;
   int progress_;
+
+  QStringList files_with_errors_;
 };
 
 #endif // ORGANISE_H

src/core/utilities.cpp

@@ -18,6 +18,7 @@
 
 #include <QCoreApplication>
 #include <QDir>
+#include <QIODevice>
 #include <QStringList>
 #include <QTemporaryFile>
 
@@ -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

src/core/utilities.h

@@ -19,6 +19,8 @@
 
 #include <QString>
 
+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

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);
+    if (!Utilities::Copy(&source_file, &dest_file))
-    dest_file.open(QIODevice::WriteOnly);
+      return false;
-    dest_file.write(source_file.readAll());
   }
 
   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() {

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);
 

src/devices/afctransfer.cpp

@@ -18,6 +18,7 @@
 #include "afctransfer.h"
 #include "imobiledeviceconnection.h"
 #include "core/taskmanager.h"
+#include "core/utilities.h"
 
 #include <QDir>
 #include <QtDebug>
@@ -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);
+  return Utilities::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;
 }

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<ConnectedDevice> device_;
   QThread* original_thread_;

src/devices/gpoddevice.cpp

@@ -129,22 +129,24 @@ bool GPodDevice::CopyToStorage(
   return true;
 }
 
-void GPodDevice::FinishCopy() {
+void GPodDevice::FinishCopy(bool success) {
-  // Write the itunes database
+  if (success) {
-  GError* error = NULL;
+    // Write the itunes database
-  itdb_write(db_, &error);
+    GError* error = NULL;
-  if (error) {
+    itdb_write(db_, &error);
-    qDebug() << "GPodDevice error:" << error->message;
+    if (error) {
-    emit Error(QString::fromUtf8(error->message));
+      qDebug() << "GPodDevice error:" << error->message;
-    g_error_free(error);
+      emit Error(QString::fromUtf8(error->message));
-  } else {
+      g_error_free(error);
-    FinaliseDatabase();
+    } else {
+      FinaliseDatabase();
 
-    // Update the library model
+      // Update the library model
-    if (!songs_to_add_.isEmpty())
+      if (!songs_to_add_.isEmpty())
-      backend_->AddOrUpdateSongs(songs_to_add_);
+        backend_->AddOrUpdateSongs(songs_to_add_);
-    if (!songs_to_remove_.isEmpty())
+      if (!songs_to_remove_.isEmpty())
-      backend_->DeleteSongs(songs_to_remove_);
+        backend_->DeleteSongs(songs_to_remove_);
+    }
   }
 
   songs_to_add_.clear();
@@ -197,7 +199,7 @@ bool GPodDevice::DeleteFromStorage(const Song& metadata) {
   return true;
 }
 
-void GPodDevice::FinishDelete() {
+void GPodDevice::FinishDelete(bool success) {
-  FinishCopy();
+  FinishCopy(success);
 }
 

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);

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_);
   }