[libmediaart/api-cleanup: 13/14] extract: Add _SYMLINK_FAILED error and report symlink() failure in get_heuristic()



commit f7fa1a81bcce17656dc039b4d5bfd46360bc0794
Author: Martyn Russell <martyn lanedo com>
Date:   Tue Feb 25 16:54:14 2014 +0000

    extract: Add _SYMLINK_FAILED error and report symlink() failure in get_heuristic()

 libmediaart/extract.c |  446 ++++++++++++++++++++++++++++++-------------------
 libmediaart/extract.h |   10 +-
 2 files changed, 280 insertions(+), 176 deletions(-)
---
diff --git a/libmediaart/extract.c b/libmediaart/extract.c
index 7af2402..8991d7a 100644
--- a/libmediaart/extract.c
+++ b/libmediaart/extract.c
@@ -393,12 +393,22 @@ convert_from_other_format (const gchar  *found,
        }
 
        if (artist == NULL || g_strcmp0 (artist, " ") == 0) {
-               if (g_rename (target_temp, album_path) == -1) {
-                       g_debug ("rename(%s, %s) error: %s",
-                                target_temp,
-                                album_path,
-                                g_strerror (errno));
+               retval = g_rename (target_temp, album_path) == 0;
+
+               if (!retval) {
+                       g_set_error (error,
+                                    MEDIA_ART_ERROR,
+                                    MEDIA_ART_ERROR_RENAME_FAILED,
+                                    "Could not rename '%s' to '%s': %s",
+                                    target_temp,
+                                    album_path,
+                                    g_strerror (errno));
                }
+
+               g_debug ("rename(%s, %s) error: %s",
+                        target_temp,
+                        album_path,
+                        g_strerror (errno));
        } else if (file_get_checksum_if_exists (G_CHECKSUM_MD5,
                                                target_temp,
                                                &sum1,
@@ -415,45 +425,77 @@ convert_from_other_format (const gchar  *found,
 
                                /* If album-space-md5.jpg is the same as found,
                                 * make a symlink */
-
-                               if (symlink (album_path, target) != 0) {
-                                       g_debug ("symlink(%s, %s) error: %s",
-                                                album_path,
-                                                target,
-                                                g_strerror (errno));
-                                       retval = FALSE;
-                               } else {
-                                       retval = TRUE;
+                               retval = symlink (album_path, target) == 0;
+
+                               if (!retval) {
+                                       g_set_error (error,
+                                                    MEDIA_ART_ERROR,
+                                                    MEDIA_ART_ERROR_RENAME_FAILED,
+                                                    "Could not rename '%s' to '%s': %s",
+                                                    album_path,
+                                                    target,
+                                                    g_strerror (errno));
                                }
 
+                               g_debug ("symlink(%s, %s) error: %s",
+                                        album_path,
+                                        target,
+                                        g_strerror (errno));
+
                                g_unlink (target_temp);
                        } else {
 
                                /* If album-space-md5.jpg isn't the same as found,
                                 * make a new album-md5-md5.jpg (found -> target) */
-
-                               if (g_rename (target_temp, album_path) == -1) {
-                                       g_debug ("rename(%s, %s) error: %s",
-                                                target_temp,
-                                                album_path,
-                                                g_strerror (errno));
+                               retval = g_rename (target_temp, album_path) == 0;
+
+                               if (!retval) {
+                                       g_set_error (error,
+                                                    MEDIA_ART_ERROR,
+                                                    MEDIA_ART_ERROR_RENAME_FAILED,
+                                                    "Could not rename '%s' to '%s': %s",
+                                                    target_temp,
+                                                    album_path,
+                                                    g_strerror (errno));
                                }
+
+                               g_debug ("rename(%s, %s) error: %s",
+                                        target_temp,
+                                        album_path,
+                                        g_strerror (errno));
                        }
 
                        g_free (sum2);
                } else {
                        /* If there's not yet a album-space-md5.jpg, make one,
                         * and symlink album-md5-md5.jpg to it */
-                       g_rename (target_temp, album_path);
+                       retval = g_rename (target_temp, album_path) == 0;
+
+                       if (!retval) {
+                               g_set_error (error,
+                                            MEDIA_ART_ERROR,
+                                            MEDIA_ART_ERROR_RENAME_FAILED,
+                                            "Could not rename '%s' to '%s': %s",
+                                            album_path,
+                                            target,
+                                            g_strerror (errno));
+                       } else {
+                               retval = symlink (album_path, target) == 0;
+
+                               if (!retval) {
+                                       g_set_error (error,
+                                                    MEDIA_ART_ERROR,
+                                                    MEDIA_ART_ERROR_SYMLINK_FAILED,
+                                                    "Could not rename '%s' to '%s': %s",
+                                                    album_path,
+                                                    target,
+                                                    g_strerror (errno));
+                               }
 
-                       if (symlink (album_path, target) != 0) {
                                g_debug ("symlink(%s,%s) error: %s",
-                                        album_path,
-                                        target,
-                                        g_strerror (errno));
-                               retval = FALSE;
-                       } else {
-                               retval = TRUE;
+                                       album_path,
+                                       target,
+                                       g_strerror (errno));
                        }
                }
 
@@ -725,173 +767,188 @@ get_heuristic (MediaArtType   type,
                                                            artist,
                                                            title);
 
-       if (art_file_path != NULL) {
-               if (g_str_has_suffix (art_file_path, "jpeg") ||
-                   g_str_has_suffix (art_file_path, "jpg")) {
-
-                       gboolean is_jpeg = FALSE;
-                       gchar *sum1 = NULL;
+       if (!art_file_path) {
+               // FIXME: Do we GError here?
 
-                       if (type != MEDIA_ART_ALBUM ||
-                           (artist == NULL || g_strcmp0 (artist, " ") == 0)) {
-                               GFile *art_file;
-                               GFile *target_file;
-                               GError *err = NULL;
-
-                               g_debug ("Album art (JPEG) found in same directory being used:'%s'",
-                                        art_file_path);
-
-                               target_file = g_file_new_for_path (target);
-                               art_file = g_file_new_for_path (art_file_path);
+               g_free (art_file_path);
+               g_free (album_art_file_path);
 
-                               g_file_copy (art_file,
-                                            target_file,
-                                            0,
-                                            NULL,
-                                            NULL,
-                                            NULL,
-                                            &err);
+               g_free (target);
+               g_free (artist_stripped);
+               g_free (title_stripped);
 
-                               if (err) {
-                                       g_debug ("%s", err->message);
-                                       g_clear_error (&err);
-                               }
+               return FALSE;
+       }
 
-                               g_object_unref (art_file);
-                               g_object_unref (target_file);
-                       } else if (file_get_checksum_if_exists (G_CHECKSUM_MD5,
-                                                               art_file_path,
-                                                               &sum1,
-                                                               TRUE,
-                                                               &is_jpeg)) {
-                               /* Avoid duplicate artwork for each track in an album */
-                               media_art_get_path (NULL,
-                                                   title_stripped,
-                                                   media_art_type_name [type],
-                                                   NULL,
-                                                   &album_art_file_path,
-                                                   NULL);
+       if (g_str_has_suffix (art_file_path, "jpeg") ||
+           g_str_has_suffix (art_file_path, "jpg")) {
+               gboolean is_jpeg = FALSE;
+               gchar *sum1 = NULL;
+
+               if (type != MEDIA_ART_ALBUM ||
+                   (artist == NULL || g_strcmp0 (artist, " ") == 0)) {
+                       GFile *art_file;
+                       GFile *target_file;
+                       GError *local_error = NULL;
+
+                       g_debug ("Album art (JPEG) found in same directory being used:'%s'",
+                                art_file_path);
+
+                       target_file = g_file_new_for_path (target);
+                       art_file = g_file_new_for_path (art_file_path);
+
+                       g_file_copy (art_file,
+                                    target_file,
+                                    0,
+                                    NULL,
+                                    NULL,
+                                    NULL,
+                                    &local_error);
+
+                       if (local_error) {
+                               g_debug ("%s", local_error->message);
+                               g_propagate_error (error, local_error);
+                       }
 
-                               if (is_jpeg) {
-                                       gchar *sum2 = NULL;
-
-                                       g_debug ("Album art (JPEG) found in same directory being used:'%s'", 
art_file_path);
-
-                                       if (file_get_checksum_if_exists (G_CHECKSUM_MD5,
-                                                                        album_art_file_path,
-                                                                        &sum2,
-                                                                        FALSE,
-                                                                        NULL)) {
-                                               if (g_strcmp0 (sum1, sum2) == 0) {
-                                                       /* If album-space-md5.jpg is the same as found,
-                                                        * make a symlink */
-
-                                                       if (symlink (album_art_file_path, target) != 0) {
-                                                               g_debug ("symlink(%s, %s) error: %s",
-                                                                        album_art_file_path,
-                                                                        target,
-                                                                        g_strerror (errno));
-                                                               retval = FALSE;
-                                                       } else {
-                                                               retval = TRUE;
-                                                       }
-                                               } else {
-                                                       GFile *art_file;
-                                                       GFile *target_file;
-                                                       GError *err = NULL;
-
-                                                       /* If album-space-md5.jpg isn't the same as found,
-                                                        * make a new album-md5-md5.jpg (found -> target) */
-
-                                                       target_file = g_file_new_for_path (target);
-                                                       art_file = g_file_new_for_path (art_file_path);
-                                                       retval = g_file_copy (art_file,
-                                                                             target_file,
-                                                                             0,
-                                                                             NULL,
-                                                                             NULL,
-                                                                             NULL,
-                                                                             &err);
-                                                       if (err) {
-                                                               g_debug ("%s", err->message);
-                                                               g_clear_error (&err);
-                                                       }
-                                                       g_object_unref (art_file);
-                                                       g_object_unref (target_file);
+                       g_object_unref (art_file);
+                       g_object_unref (target_file);
+               } else if (file_get_checksum_if_exists (G_CHECKSUM_MD5,
+                                                       art_file_path,
+                                                       &sum1,
+                                                       TRUE,
+                                                       &is_jpeg)) {
+                       /* Avoid duplicate artwork for each track in an album */
+                       media_art_get_path (NULL,
+                                           title_stripped,
+                                           media_art_type_name [type],
+                                           NULL,
+                                           &album_art_file_path,
+                                           NULL);
+
+                       if (is_jpeg) {
+                               gchar *sum2 = NULL;
+
+                               g_debug ("Album art (JPEG) found in same directory being used:'%s'", 
art_file_path);
+
+                               if (file_get_checksum_if_exists (G_CHECKSUM_MD5,
+                                                                album_art_file_path,
+                                                                &sum2,
+                                                                FALSE,
+                                                                NULL)) {
+                                       if (g_strcmp0 (sum1, sum2) == 0) {
+                                               /* If album-space-md5.jpg is the same as found,
+                                                * make a symlink */
+                                               retval = symlink (album_art_file_path, target) == 0;
+
+                                               if (!retval) {
+                                                       g_set_error (error,
+                                                                    MEDIA_ART_ERROR,
+                                                                    MEDIA_ART_ERROR_SYMLINK_FAILED,
+                                                                    "Could not link '%s' to '%s': %s",
+                                                                    album_art_file_path,
+                                                                    target,
+                                                                    g_strerror (errno));
                                                }
-                                               g_free (sum2);
+
+                                               g_debug ("symlink(%s, %s) error: %s",
+                                                        album_art_file_path,
+                                                        target,
+                                                        g_strerror (errno));
                                        } else {
                                                GFile *art_file;
-                                               GFile *album_art_file;
-                                               GError *err = NULL;
-
-                                               /* If there's not yet a album-space-md5.jpg, make one,
-                                                * and symlink album-md5-md5.jpg to it */
+                                               GFile *target_file;
+                                               GError *local_error = NULL;
 
-                                               album_art_file = g_file_new_for_path (album_art_file_path);
+                                               /* If album-space-md5.jpg isn't the same as found,
+                                                * make a new album-md5-md5.jpg (found -> target) */
+                                               target_file = g_file_new_for_path (target);
                                                art_file = g_file_new_for_path (art_file_path);
                                                retval = g_file_copy (art_file,
-                                                                     album_art_file,
+                                                                     target_file,
                                                                      0,
                                                                      NULL,
                                                                      NULL,
                                                                      NULL,
-                                                                     &err);
-
-                                               if (err == NULL) {
-                                                       if (symlink (album_art_file_path, target) != 0) {
-                                                               g_debug ("symlink(%s, %s) error: %s",
-                                                                        album_art_file_path, target,
-                                                                        g_strerror (errno));
-                                                               retval = FALSE;
-                                                       } else {
-                                                               retval = TRUE;
-                                                       }
-                                               } else {
-                                                       g_debug ("%s", err->message);
-                                                       g_clear_error (&err);
-                                                       retval = FALSE;
-                                               }
+                                                                     error);
 
-                                               g_object_unref (album_art_file);
                                                g_object_unref (art_file);
+                                               g_object_unref (target_file);
                                        }
+
+                                       g_free (sum2);
                                } else {
-                                       g_debug ("Album art found in same directory but not a real JPEG file 
(trying to convert): '%s'", art_file_path);
-                                       retval = convert_from_other_format (art_file_path,
-                                                                           target,
-                                                                           album_art_file_path,
-                                                                           artist,
-                                                                           error);
-                               }
+                                       GFile *art_file;
+                                       GFile *album_art_file;
+                                       GError *local_error = NULL;
+
+                                       /* If there's not yet a album-space-md5.jpg, make one,
+                                        * and symlink album-md5-md5.jpg to it */
+                                       album_art_file = g_file_new_for_path (album_art_file_path);
+                                       art_file = g_file_new_for_path (art_file_path);
+                                       retval = g_file_copy (art_file,
+                                                             album_art_file,
+                                                             0,
+                                                             NULL,
+                                                             NULL,
+                                                             NULL,
+                                                             error);
+
+                                       if (retval) {
+                                               retval = symlink (album_art_file_path, target) == 0;
+
+                                               if (!retval) {
+                                                       g_set_error (error,
+                                                                    MEDIA_ART_ERROR,
+                                                                    MEDIA_ART_ERROR_SYMLINK_FAILED,
+                                                                    "Could not link '%s' to '%s': %s",
+                                                                    album_art_file_path,
+                                                                    target,
+                                                                    g_strerror (errno));
+                                               }
 
-                               g_free (sum1);
+                                               g_debug ("symlink(%s, %s) error: %s",
+                                                        album_art_file_path, target,
+                                                        g_strerror (errno));
+                                       }
+
+                                       g_object_unref (album_art_file);
+                                       g_object_unref (art_file);
+                               }
                        } else {
-                               /* Can't read contents of the cover.jpg file ... */
-                               retval = FALSE;
-                       }
-               } else if (g_str_has_suffix (art_file_path, "png")) {
-                       if (!album_art_file_path) {
-                               media_art_get_path (NULL,
-                                                   title_stripped,
-                                                   media_art_type_name[type],
-                                                   NULL,
-                                                   &album_art_file_path,
-                                                   NULL);
+                               g_debug ("Album art found in same directory but not a real JPEG file (trying 
to convert): '%s'", art_file_path);
+                               retval = convert_from_other_format (art_file_path,
+                                                                   target,
+                                                                   album_art_file_path,
+                                                                   artist,
+                                                                   error);
                        }
 
-                       g_debug ("Album art (PNG) found in same directory being used:'%s'", art_file_path);
-                       retval = convert_from_other_format (art_file_path,
-                                                           target,
-                                                           album_art_file_path,
-                                                           artist,
-                                                           error);
+                       g_free (sum1);
+               } else {
+                       /* Can't read contents of the cover.jpg file ... */
+                       retval = FALSE;
+               }
+       } else if (g_str_has_suffix (art_file_path, "png")) {
+               if (!album_art_file_path) {
+                       media_art_get_path (NULL,
+                                           title_stripped,
+                                           media_art_type_name[type],
+                                           NULL,
+                                           &album_art_file_path,
+                                           NULL);
                }
 
-               g_free (art_file_path);
-               g_free (album_art_file_path);
+               g_debug ("Album art (PNG) found in same directory being used:'%s'", art_file_path);
+               retval = convert_from_other_format (art_file_path,
+                                                   target,
+                                                   album_art_file_path,
+                                                   artist,
+                                                   error);
        }
 
+       g_free (art_file_path);
+       g_free (album_art_file_path);
+
        g_free (target);
        g_free (artist_stripped);
        g_free (title_stripped);
@@ -1010,14 +1067,22 @@ media_art_set (const unsigned char  *buffer,
                         * exist, make one and make a symlink
                         * to album-md5-md5.jpg
                         */
-                       if (symlink (album_path, local_path) != 0) {
-                               g_debug ("Creating symlink '%s' --> '%s', %s",
-                                        album_path,
-                                        local_path,
-                                        retval ? g_strerror (errno) : "no error given");
+                       retval = symlink (album_path, local_path) == 0;
 
-                               retval = FALSE;
+                       if (!retval) {
+                               g_set_error (error,
+                                            MEDIA_ART_ERROR,
+                                            MEDIA_ART_ERROR_SYMLINK_FAILED,
+                                            "Could not link '%s' to '%s': %s",
+                                            album_path,
+                                            local_path,
+                                            g_strerror (errno));
                        }
+
+                       g_debug ("Creating symlink '%s' --> '%s', %s",
+                                album_path,
+                                local_path,
+                                retval ? g_strerror (errno) : "no error given");
                }
 
                g_free (album_path);
@@ -1054,6 +1119,17 @@ media_art_set (const unsigned char  *buffer,
                 */
                if (g_strcmp0 (md5_data, md5_album) == 0) {
                        retval = symlink (album_path, local_path) == 0;
+
+                       if (!retval) {
+                               g_set_error (error,
+                                            MEDIA_ART_ERROR,
+                                            MEDIA_ART_ERROR_SYMLINK_FAILED,
+                                            "Could not link '%s' to '%s': %s",
+                                            album_path,
+                                            local_path,
+                                            g_strerror (errno));
+                       }
+
                        g_debug ("Creating symlink '%s' --> '%s', %s",
                                 album_path,
                                 local_path,
@@ -1105,6 +1181,17 @@ media_art_set (const unsigned char  *buffer,
                         * buffer, make a symlink to album-md5-md5.jpg
                         */
                        retval = symlink (album_path, local_path) == 0;
+
+                       if (!retval) {
+                               g_set_error (error,
+                                            MEDIA_ART_ERROR,
+                                            MEDIA_ART_ERROR_SYMLINK_FAILED,
+                                            "Could not link '%s' to '%s': %s",
+                                            album_path,
+                                            local_path,
+                                            g_strerror (errno));
+                       }
+
                        g_debug ("Creating symlink '%s' --> '%s', %s",
                                 album_path,
                                 local_path,
@@ -1114,6 +1201,17 @@ media_art_set (const unsigned char  *buffer,
                         * buffer, make a new album-md5-md5.jpg
                         */
                        retval = g_rename (temp, local_path) == 0;
+
+                       if (!retval) {
+                               g_set_error (error,
+                                            MEDIA_ART_ERROR,
+                                            MEDIA_ART_ERROR_RENAME_FAILED,
+                                            "Could not rename '%s' to '%s': %s",
+                                            temp,
+                                            local_path,
+                                            g_strerror (errno));
+                       }
+
                        g_debug ("Renaming temp file '%s' --> '%s', %s",
                                 temp,
                                 local_path,
diff --git a/libmediaart/extract.h b/libmediaart/extract.h
index 68dd308..a17c8c9 100644
--- a/libmediaart/extract.h
+++ b/libmediaart/extract.h
@@ -51,7 +51,11 @@ typedef enum {
  * MediaArtError:
  * @MEDIA_ART_ERROR_NO_STORAGE: Storage information is unknown, we
  * have no knowledge about removable media.
- * @MEDIA_ART_ERROR_NO_TITLE: Title is required, but was not provided, or was empty.
+ * @MEDIA_ART_ERROR_NO_TITLE: Title is required, but was not provided,
+ * or was empty.
+ * @MEDIA_ART_ERROR_SYMLINK_FAILED: A call to symlink() failed
+ * resulting in the incorrect storage of media art.
+ * @MEDIA_ART_ERROR_RENAME_FAILED: File could not be renamed.
  *
  * Enumeration values used in errors returned by the
  * #MediaArtError API.
@@ -60,7 +64,9 @@ typedef enum {
  **/
 typedef enum {
        MEDIA_ART_ERROR_NO_STORAGE,
-       MEDIA_ART_ERROR_NO_TITLE
+       MEDIA_ART_ERROR_NO_TITLE,
+       MEDIA_ART_ERROR_SYMLINK_FAILED,
+       MEDIA_ART_ERROR_RENAME_FAILED
 } MediaArtError;
 
 #define MEDIA_ART_ERROR media_art_error_quark ()


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]