From 0d231f230f4fa0851d0cc05d5133b167f35539b5 Mon Sep 17 00:00:00 2001 From: Martyn Russell Date: Tue, 25 Feb 2014 16:54:14 +0000 Subject: 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 ef04df8..48f1476 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 () -- cgit v1.2.1