[easytag/wip/gerror: 3/3] WIP Use GError in ID3 tagging code
- From: David King <davidk src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [easytag/wip/gerror: 3/3] WIP Use GError in ID3 tagging code
- Date: Tue, 23 Sep 2014 23:18:34 +0000 (UTC)
commit 23bba0455ea7e85014babd1b953262154dbb87b0
Author: David King <amigadave amigadave com>
Date: Wed Sep 24 00:17:05 2014 +0100
WIP Use GError in ID3 tagging code
TODO: Return soft errors from ID3 code. Set the GError when returning
FALSE.
src/et_core.c | 8 +++-
src/tags/flac_tag.c | 4 +-
src/tags/id3_tag.c | 60 ++++++++++++++++------------
src/tags/id3_tag.h | 6 +-
src/tags/id3v24_tag.c | 102 ++++++++++++++++++++++++++-----------------------
5 files changed, 99 insertions(+), 81 deletions(-)
---
diff --git a/src/et_core.c b/src/et_core.c
index c7edbfa..e5986c5 100644
--- a/src/et_core.c
+++ b/src/et_core.c
@@ -530,7 +530,11 @@ GList *ET_Add_File_To_File_List (gchar *filename)
{
#ifdef ENABLE_MP3
case ID3_TAG:
- Id3tag_Read_File_Tag(filename,FileTag);
+ if (!id3tag_read_file_tag (filename, FileTag, &error))
+ {
+ Log_Print (LOG_ERROR, "%s", error->message);
+ g_clear_error (&error);
+ }
break;
#endif
#ifdef ENABLE_OGG
@@ -3425,7 +3429,7 @@ ET_Save_File_Tag_To_HD (ET_File *ETFile, GError **error)
{
#ifdef ENABLE_MP3
case ID3_TAG:
- state = Id3tag_Write_File_Tag(ETFile);
+ state = id3tag_write_file_tag (ETFile, error);
break;
#endif
#ifdef ENABLE_OGG
diff --git a/src/tags/flac_tag.c b/src/tags/flac_tag.c
index 8976d91..8478c2b 100644
--- a/src/tags/flac_tag.c
+++ b/src/tags/flac_tag.c
@@ -736,7 +736,7 @@ flac_tag_read_file_tag (const gchar *filename,
&& FileTag->encoded_by == NULL
&& FileTag->picture == NULL)
{
- gint rc = Id3tag_Read_File_Tag(filename,FileTag);
+ gint rc = id3tag_read_file_tag (filename, FileTag, NULL);
// If an ID3 tag has been found (and no FLAC tag), we mark the file as
// unsaved to rewrite a flac tag.
@@ -1156,7 +1156,7 @@ flac_tag_write_file_tag (ET_File *ETFile, GError **error)
// With empty tag...
ETFile_tmp->FileTagList = g_list_append(NULL,FileTag_tmp);
ETFile_tmp->FileTag = ETFile_tmp->FileTagList;
- Id3tag_Write_File_Tag(ETFile_tmp);
+ id3tag_write_file_tag (ETFile_tmp, NULL);
ET_Free_File_List_Item(ETFile_tmp);
}
#endif
diff --git a/src/tags/id3_tag.c b/src/tags/id3_tag.c
index c4d00b9..9a13564 100644
--- a/src/tags/id3_tag.c
+++ b/src/tags/id3_tag.c
@@ -135,20 +135,19 @@ et_id3tag_get_tpos_from_file_tag (File_Tag *FileTag)
* Write the ID3 tags to the file. Returns TRUE on success, else 0.
*/
static gboolean
-Id3tag_Write_File_v23Tag (ET_File *ETFile)
+id3tag_write_file_v23tag (ET_File *ETFile, GError **error)
{
File_Tag *FileTag;
gchar *filename;
gchar *filename_utf8;
gchar *basename_utf8;
GFile *file;
- GError *gerror = NULL;
ID3Tag *id3_tag = NULL;
ID3_Err error_strip_id3v1 = ID3E_NoError;
ID3_Err error_strip_id3v2 = ID3E_NoError;
ID3_Err error_update_id3v1 = ID3E_NoError;
ID3_Err error_update_id3v2 = ID3E_NoError;
- gint error = 0;
+ gboolean success = TRUE;
gint number_of_frames;
gboolean has_title = FALSE;
gboolean has_artist = FALSE;
@@ -176,6 +175,7 @@ Id3tag_Write_File_v23Tag (ET_File *ETFile)
Picture *pic;
g_return_val_if_fail (ETFile != NULL && ETFile->FileTag != NULL, FALSE);
+ g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
// When writing the first MP3 file, we check if the version of id3lib of the
// system doesn't contain a bug when writting Unicode tags
@@ -192,21 +192,15 @@ Id3tag_Write_File_v23Tag (ET_File *ETFile)
file = g_file_new_for_path (filename);
+ /* FIXME: Handle this in the caller instead. */
/* This is a protection against a bug in id3lib that enters an infinite
* loop with corrupted MP3 files (files containing only zeroes) */
- if (et_id3tag_check_if_file_is_corrupted (file, &gerror))
+ if (et_id3tag_check_if_file_is_corrupted (file, error))
{
GtkWidget *msgdialog;
gchar *basename;
gchar *basename_utf8;
- if (gerror)
- {
- Log_Print (LOG_ERROR, _("Error while reading file ‘%s’: %s"),
- filename_utf8, gerror->message);
- g_error_free (gerror);
- }
-
basename = g_file_get_basename (file);
basename_utf8 = filename_to_display (basename);
@@ -229,8 +223,12 @@ Id3tag_Write_File_v23Tag (ET_File *ETFile)
/* We get again the tag from the file to keep also unused data (by EasyTAG), then
* we replace the changed data */
- if ( (id3_tag = ID3Tag_New()) == NULL )
+ if ((id3_tag = ID3Tag_New ()) == NULL)
+ {
+ g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_NOMEM,
+ g_strerror (ENOMEM));
return FALSE;
+ }
basename_utf8 = g_path_get_basename(filename_utf8);
@@ -614,7 +612,8 @@ Id3tag_Write_File_v23Tag (ET_File *ETFile)
Id3tag_Get_Error_Message (error_strip_id3v2));
}
- error++;
+ /* FIXME: Pass the soft error to the caller. */
+ success = FALSE;
}
}else
@@ -637,7 +636,8 @@ Id3tag_Write_File_v23Tag (ET_File *ETFile)
_("Error while updating ID3v2 tag of ‘%s’: %s"),
basename_utf8,
Id3tag_Get_Error_Message (error_update_id3v2));
- error++;
+ /* FIXME: Pass the soft error to the caller. */
+ success = FALSE;
}else
{
/* See known problem on the top : [ 1016290 ] Unicode16 writing bug.
@@ -653,7 +653,7 @@ Id3tag_Write_File_v23Tag (ET_File *ETFile)
"id3v2-enable-unicode"))
{
File_Tag *FileTag_tmp = ET_File_Tag_Item_New();
- if (Id3tag_Read_File_Tag(filename,FileTag_tmp) == TRUE
+ if (id3tag_read_file_tag (filename, FileTag_tmp, NULL) == TRUE
&& ET_Detect_Changes_Of_File_Tag(FileTag,FileTag_tmp) == TRUE)
{
GtkWidget *msgdialog;
@@ -692,7 +692,8 @@ Id3tag_Write_File_v23Tag (ET_File *ETFile)
_("Error while removing ID3v2 tag of ‘%s’: %s"),
basename_utf8,
Id3tag_Get_Error_Message (error_strip_id3v2));
- error++;
+ /* FIXME: Pass the soft error to the caller. */
+ success = FALSE;
}
}
@@ -716,7 +717,8 @@ Id3tag_Write_File_v23Tag (ET_File *ETFile)
_("Error while updating ID3v1 tag of ‘%s’: %s"),
basename_utf8,
Id3tag_Get_Error_Message (error_update_id3v1));
- error++;
+ /* FIXME: Pass the soft error to the caller. */
+ success = FALSE;
}
}else
{
@@ -727,11 +729,12 @@ Id3tag_Write_File_v23Tag (ET_File *ETFile)
_("Error while removing ID3v1 tag of ‘%s’: %s"),
basename_utf8,
Id3tag_Get_Error_Message (error_strip_id3v1));
- error++;
+ /* FIXME: Pass the soft error to the caller. */
+ success = FALSE;
}
}
- if (error == 0)
+ if (success)
{
Log_Print (LOG_OK, _("Updated tag of ‘%s’"), basename_utf8);
}
@@ -742,9 +745,8 @@ Id3tag_Write_File_v23Tag (ET_File *ETFile)
ID3Tag_Delete(id3_tag);
g_free(basename_utf8);
- if (error) return FALSE;
- else return TRUE;
-
+ /* FIXME: set GError if success == FALSE. */
+ return success;
}
@@ -1547,15 +1549,21 @@ gboolean Id3tag_Check_If_Id3lib_Is_Bugged (void)
/*
* Write tag according the version selected by the user
*/
-gboolean Id3tag_Write_File_Tag (ET_File *ETFile)
+gboolean
+id3tag_write_file_tag (ET_File *ETFile,
+ GError **error)
{
#ifdef ENABLE_ID3LIB
if (g_settings_get_boolean (MainSettings, "id3v2-version-4"))
- return Id3tag_Write_File_v24Tag(ETFile);
+ {
+ return id3tag_write_file_v24tag (ETFile, error);
+ }
else
- return Id3tag_Write_File_v23Tag(ETFile);
+ {
+ return id3tag_write_file_v23tag (ETFile, error);
+ }
#else
- return Id3tag_Write_File_v24Tag(ETFile);
+ return id3tag_write_file_v24tag (ETFile, error);
#endif /* !ENABLE_ID3LIB */
}
diff --git a/src/tags/id3_tag.h b/src/tags/id3_tag.h
index a167ca5..b5d2f49 100644
--- a/src/tags/id3_tag.h
+++ b/src/tags/id3_tag.h
@@ -27,9 +27,9 @@ G_BEGIN_DECLS
#define ID3_INVALID_GENRE 255
-gboolean Id3tag_Read_File_Tag (const gchar *filename, File_Tag *FileTag);
-gboolean Id3tag_Write_File_v24Tag (ET_File *ETFile);
-gboolean Id3tag_Write_File_Tag (ET_File *ETFile);
+gboolean id3tag_read_file_tag (const gchar *filename, File_Tag *FileTag, GError **error);
+gboolean id3tag_write_file_v24tag (ET_File *ETFile, GError **error);
+gboolean id3tag_write_file_tag (ET_File *ETFile, GError **error);
const gchar * Id3tag_Genre_To_String (unsigned char genre_code);
guchar Id3tag_String_To_Genre (const gchar *genre);
diff --git a/src/tags/id3v24_tag.c b/src/tags/id3v24_tag.c
index 1c59ff9..a4a527a 100644
--- a/src/tags/id3v24_tag.c
+++ b/src/tags/id3v24_tag.c
@@ -78,8 +78,8 @@ static void Id3tag_delete_txxframes (struct id3_tag *tag, const gchar *param
static struct id3_frame *Id3tag_find_and_create_frame (struct id3_tag *tag, const gchar *name);
static int id3taglib_set_field (struct id3_frame *frame, const gchar *str, enum id3_field_type
type, int num, int clear, int id3v1);
static int etag_set_tags (const gchar *str, const char *frame_name, enum id3_field_type
field_type, struct id3_tag *v1tag, struct id3_tag *v2tag, gboolean *strip_tags);
-static int etag_write_tags (const gchar *filename, struct id3_tag const *v1tag,
- struct id3_tag const *v2tag, gboolean strip_tags);
+static gboolean etag_write_tags (const gchar *filename, struct id3_tag const *v1tag,
+ struct id3_tag const *v2tag, gboolean strip_tags, GError **error);
/*************
* Functions *
@@ -91,8 +91,9 @@ static int etag_write_tags (const gchar *filename, struct id3_tag const *v1tag,
* If a tag entry exists (ex: title), we allocate memory, else value stays to NULL
*/
gboolean
-Id3tag_Read_File_Tag (const gchar *filename,
- File_Tag *FileTag)
+id3tag_read_file_tag (const gchar *filename,
+ File_Tag *FileTag,
+ GError **error)
{
int tmpfile;
struct id3_file *file;
@@ -105,15 +106,16 @@ Id3tag_Read_File_Tag (const gchar *filename,
unsigned tmpupdate, update = 0;
long tagsize;
-
g_return_val_if_fail (filename != NULL && FileTag != NULL, FALSE);
+ g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
- if ( (tmpfile=open(filename,O_RDONLY)) < 0 )
+ if ((tmpfile = open (filename, O_RDONLY)) < 0)
{
- gchar *filename_utf8 = filename_to_display(filename);
- Log_Print (LOG_ERROR, _("Error while opening file ‘%s’: %s"),
- filename_utf8, g_strerror (errno));
- g_free(filename_utf8);
+ gchar *filename_utf8 = filename_to_display (filename);
+ g_set_error (error, G_FILE_ERROR, g_file_error_from_errno (errno),
+ _("Error while opening file ‘%s’: %s"),
+ filename_utf8, g_strerror (errno));
+ g_free (filename_utf8);
return FALSE;
}
@@ -872,7 +874,8 @@ libid3tag_Get_Frame_Str (const struct id3_frame *frame,
/*
* Write the ID3 tags to the file. Returns TRUE on success, else 0.
*/
-gboolean Id3tag_Write_File_v24Tag (ET_File *ETFile)
+gboolean
+id3tag_write_file_v24tag (ET_File *ETFile, GError **error)
{
File_Tag *FileTag;
gchar *filename, *filename_utf8;
@@ -882,11 +885,12 @@ gboolean Id3tag_Write_File_v24Tag (ET_File *ETFile)
union id3_field *field;
gchar *string1;
Picture *pic;
- gint error = 0;
gboolean strip_tags = TRUE;
guchar genre_value = ID3_INVALID_GENRE;
+ gboolean success;
g_return_val_if_fail (ETFile != NULL && ETFile->FileTag != NULL, FALSE);
+ g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
FileTag = (File_Tag *)ETFile->FileTag->data;
filename = ((File_Name *)ETFile->FileNameCur->data)->value;
@@ -1138,7 +1142,7 @@ gboolean Id3tag_Write_File_v24Tag (ET_File *ETFile)
/*********************************
* Update id3v1.x and id3v2 tags *
*********************************/
- error |= etag_write_tags(filename, v1tag, v2tag, strip_tags);
+ success = etag_write_tags (filename, v1tag, v2tag, strip_tags, error);
// Free data
if (v1tag)
@@ -1146,16 +1150,14 @@ gboolean Id3tag_Write_File_v24Tag (ET_File *ETFile)
if (v2tag)
id3_tag_delete(v2tag);
- if (error == 0)
+ if (success)
{
basename_utf8 = g_path_get_basename(filename_utf8);
Log_Print (LOG_OK, _("Updated tag of ‘%s’"), basename_utf8);
g_free(basename_utf8);
}
- if (error) return FALSE;
- else return TRUE;
-
+ return success;
}
/* Dele all frames with 'name'
@@ -1463,11 +1465,12 @@ etag_set_tags (const gchar *str,
return 0;
}
-static int
+static gboolean
etag_write_tags (const gchar *filename,
struct id3_tag const *v1tag,
struct id3_tag const *v2tag,
- gboolean strip_tags)
+ gboolean strip_tags,
+ GError **error)
{
id3_byte_t *v1buf, *v2buf;
id3_length_t v1size = 0, v2size = 0;
@@ -1477,7 +1480,7 @@ etag_write_tags (const gchar *filename,
long filev2size;
gsize ctxsize;
char *ctx = NULL;
- int err = 0;
+ gboolean success = TRUE;
gssize size_read = 0;
v1buf = v2buf = NULL;
@@ -1523,13 +1526,14 @@ etag_write_tags (const gchar *filename,
if ((fd = open(filename, O_RDWR)) < 0)
{
- err = errno;
- g_free(v1buf);
- g_free(v2buf);
- return (err);
+ g_set_error (error, G_FILE_ERROR, g_file_error_from_errno (errno),
+ "%s", g_strerror (errno));
+ g_free (v1buf);
+ g_free (v2buf);
+ return FALSE;
}
- err = 1;
+ success = FALSE;
/* Handle Id3v1 tag */
if ((curpos = lseek(fd, -128, SEEK_END)) < 0)
@@ -1587,7 +1591,7 @@ etag_write_tags (const gchar *filename,
/* Truncate file (strip tags at the end of file) */
if ((curpos = lseek(fd, 0, SEEK_CUR)) <= 0 )
goto out;
- if ((err = ftruncate(fd, curpos)))
+ if (ftruncate (fd, curpos) == -1)
goto out;
/* Handle Id3v2 tag */
@@ -1633,18 +1637,18 @@ etag_write_tags (const gchar *filename,
* support macros in extracted strings.
* https://bugzilla.gnome.org/show_bug.cgi?id=705952
*/
- Log_Print (LOG_ERROR,
- /* Translators: The first string is a filename, the
- * second string is the number of bytes that were
- * missing (not read for some reason) while reading from
- * the file.
- */
- ngettext ("Cannot write tag of file ‘%s’ (a byte was missing)",
- "Cannot write tag of file ‘%s’ (%s bytes were missing)",
- ctxsize - size_read),
- basename_utf8, bytes_missing);
- g_free(filename_utf8);
- g_free(basename_utf8);
+ g_set_error (error, G_FILE_ERROR, g_file_error_from_errno (errno),
+ /* Translators: The first string is a filename, the
+ * second string is the number of bytes that were
+ * missing (not read for some reason) while reading
+ * from the file.
+ */
+ ngettext ("Cannot write tag of file ‘%s’ (a byte was missing)",
+ "Cannot write tag of file ‘%s’ (%s bytes were missing)",
+ ctxsize - size_read),
+ basename_utf8, bytes_missing);
+ g_free (filename_utf8);
+ g_free (basename_utf8);
g_free (bytes_missing);
goto out;
}
@@ -1660,8 +1664,9 @@ etag_write_tags (const gchar *filename,
{
gchar *filename_utf8 = filename_to_display (filename);
gchar *basename_utf8 = g_path_get_basename (filename_utf8);
- Log_Print (LOG_ERROR, _("Cannot save tag of file ‘%s’"),
- basename_utf8);
+ g_set_error (error, G_FILE_ERROR,
+ g_file_error_from_errno (errno),
+ _("Cannot save tag of file ‘%s’"), basename_utf8);
g_free (basename_utf8);
goto out;
}
@@ -1671,29 +1676,30 @@ etag_write_tags (const gchar *filename,
{
gchar *filename_utf8 = filename_to_display(filename);
gchar *basename_utf8 = g_path_get_basename(filename_utf8);
- Log_Print (LOG_ERROR,
- _("Size error while saving tag of ‘%s’"),
- basename_utf8);
- g_free(filename_utf8);
- g_free(basename_utf8);
+ g_set_error (error, G_FILE_ERROR,
+ g_file_error_from_errno (errno),
+ _("Size error while saving tag of ‘%s’"),
+ basename_utf8);
+ g_free (filename_utf8);
+ g_free (basename_utf8);
goto out;
}
if ((curpos = lseek(fd, 0, SEEK_CUR)) <= 0 )
goto out;
- if ((err = ftruncate(fd, curpos)))
+ if (ftruncate (fd, curpos) == -1)
goto out;
}
- err = 0;
+ success = TRUE;
out:
g_free(ctx);
lseek(fd, 0, SEEK_SET);
close(fd);
g_free(v1buf);
g_free(v2buf);
- return err;
+ return success;
}
#endif /* ENABLE_MP3 */
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
