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

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