Reworked error handling.

vcedit functions now return an error code instead of setting lasterror.
Ogg::Vorbis::Tagger turns these error codes into their own exception
classes.

diff --git a/ext/ext.c b/ext/ext.c
index 9c433f285523c12196501363e132a8662f9907e5..67fa6b14b24811514db40966ca373d1050c9bbf8 100644 (file)
--- a/ext/ext.c
+++ b/ext/ext.c
@@ -20,6 +20,7 @@
 #include <stdio.h>
 #include <stdbool.h>
 #include <errno.h>
+#include <assert.h>
 
 #include "vcedit.h"
 #include "comments.h"
@@ -31,7 +32,8 @@ typedef struct {
 
 static VALUE c_close (VALUE self);
 
-static VALUE cComments, eVTError;
+static VALUE cComments, eVT, eOpen, eInvalidData, eInvalidComment,
+             eTempFile, eReopen;
 static ID id_length;
 
 static void
@@ -100,17 +102,23 @@ c_init (VALUE self, VALUE filename)
 
        o->state = vcedit_state_new (StringValuePtr (filename));
        if (!o->state)
-               rb_raise (eVTError, "vcedit_new_state() failed - %s",
-                         vcedit_error (o->state));
-
-       if (vcedit_open (o->state) < 0)
-               rb_raise (eVTError, "vcedit_open() failed - %s",
-                         vcedit_error (o->state));
+               rb_raise (rb_eNoMemError, "Out of Memory");
+
+       switch (vcedit_open (o->state)) {
+               case VCEDIT_ERR_OPEN:
+                       rb_raise (eOpen, "Cannot open file");
+               case VCEDIT_ERR_INVAL:
+                       rb_raise (eInvalidData, "Invalid data");
+               default:
+                       break;
+       }
 
        vc = vcedit_comments (o->state);
-       if (!vc)
-               rb_raise (eVTError, "vcedit_comments() failed - %s",
-                         vcedit_error (o->state));
+
+       /* vcedit_open() succeeded, so vcedit_comments() cannot
+        * return NULL.
+        */
+       assert (vc);
 
        /* check whether all comments are well-formed */
        for (i = 0; i < vc->comments; i++) {
@@ -118,7 +126,7 @@ c_init (VALUE self, VALUE filename)
 
                ptr = strchr (content, '=');
                if (!ptr || ptr == content)
-                       rb_raise (eVTError, "malformed comment - %s", content);
+                       rb_raise (eInvalidComment, "invalid comment - %s", content);
        }
 
        o->comments = rb_class_new_instance (0, NULL, cComments);
@@ -163,9 +171,16 @@ c_write (VALUE self)
 
        comments_sync (o->comments, o->state);
 
-       if (vcedit_write (o->state) < 0)
-               rb_raise (rb_eIOError, "write failed - %s",
-                         vcedit_error (o->state));
+       switch (vcedit_write (o->state)) {
+               case VCEDIT_ERR_INVAL:
+                       rb_raise (eInvalidData, "Invalid data");
+               case VCEDIT_ERR_TMPFILE:
+                       rb_raise (eTempFile, "Cannot create temporary file");
+               case VCEDIT_ERR_REOPEN:
+                       rb_raise (eReopen, "Cannot reopen file");
+               default:
+                       break;
+       }
 
        return rb_funcall (o->comments, id_length, 0);
 }
@@ -209,7 +224,13 @@ Init_vorbistagger_ext (void)
        rb_define_method (cVT, "write", c_write, 0);
        rb_define_method (cVT, "comments", c_comments, 0);
 
-       eVTError = rb_define_class_under (cVT, "TaggerError", eVorbis);
+       eVT = rb_define_class_under (cVT, "TaggerError", eVorbis);
+       eOpen = rb_define_class_under (cVT, "OpenError", eVT);
+       eInvalidData = rb_define_class_under (cVT, "InvalidDataError", eVT);
+       eInvalidComment = rb_define_class_under (cVT, "InvalidCommentError",
+                                                eInvalidData);
+       eTempFile = rb_define_class_under (cVT, "TempFileError", eVT);
+       eReopen = rb_define_class_under (cVT, "ReopenError", eVT);
 
        id_length = rb_intern ("length");
 

diff --git a/ext/vcedit.c b/ext/vcedit.c
index 10746706f71c97fbd0c620b4ca2d95f4d8d8be9b..22a8271afabfc84f559eddec87ad1c2042b0b0b1 100644 (file)
--- a/ext/vcedit.c
+++ b/ext/vcedit.c
@@ -43,12 +43,12 @@ struct vcedit_state_St {
        char filename[PATH_MAX];
 
        FILE *in;
+       bool opened;
        long serial;
        unsigned char *mainbuf;
        unsigned char *bookbuf;
        int     mainlen;
        int     booklen;
-       const char *lasterror;
        char *vendor;
        int prevW;
        int extrapage;
@@ -120,16 +120,10 @@ vcedit_state_new (const char *filename)
        return state;
 }
 
-const char *
-vcedit_error (vcedit_state *state)
-{
-       return state->lasterror;
-}
-
 vorbis_comment *
 vcedit_comments (vcedit_state *state)
 {
-       return state->vc;
+       return state->opened ? state->vc : NULL;
 }
 
 static void
@@ -153,6 +147,7 @@ vcedit_clear_internals (vcedit_state *state)
        state->booklen = 0;
 
        state->serial = 0;
+       state->opened = false;
 }
 
 void
@@ -278,9 +273,10 @@ _fetch_next_packet (vcedit_state *s, ogg_packet *p, ogg_page *page)
        return _fetch_next_packet (s, p, page);
 }
 
-int
+vcedit_error
 vcedit_open (vcedit_state *state)
 {
+       vcedit_error ret;
        char *buffer;
        int bytes, i;
        int chunks = 0;
@@ -289,10 +285,8 @@ vcedit_open (vcedit_state *state)
        ogg_page og;
 
        state->in = fopen (state->filename, "rb");
-       if (!state->in) {
-               state->lasterror = "Cannot open file.";
-               return -1;
-       }
+       if (!state->in)
+               return VCEDIT_ERR_OPEN;
 
        ogg_sync_init (state->oy);
 
@@ -307,10 +301,7 @@ vcedit_open (vcedit_state *state)
 
                /* Bail if we don't find data in the first 40 kB */
                if (chunks++ >= 10) {
-                       if (bytes < CHUNKSIZE)
-                               state->lasterror = "Input truncated or empty.";
-                       else
-                               state->lasterror = "Input is not an Ogg bitstream.";
+                       ret = VCEDIT_ERR_INVAL;
 
                        goto err;
                }
@@ -323,17 +314,17 @@ vcedit_open (vcedit_state *state)
        vorbis_comment_init (state->vc);
 
        if (ogg_stream_pagein (state->os, &og) < 0) {
-               state->lasterror = "Error reading first page of Ogg bitstream.";
+               ret = VCEDIT_ERR_INVAL;
                goto err;
        }
 
        if (ogg_stream_packetout (state->os, &header_main) != 1) {
-               state->lasterror = "Error reading initial header packet.";
+               ret = VCEDIT_ERR_INVAL;
                goto err;
        }
 
        if (vorbis_synthesis_headerin (state->vi, state->vc, &header_main) < 0) {
-               state->lasterror = "Ogg bitstream does not contain vorbis data.";
+               ret = VCEDIT_ERR_INVAL;
                goto err;
        }
 
@@ -361,7 +352,7 @@ vcedit_open (vcedit_state *state)
                                                break;
 
                                        if (result == -1) {
-                                               state->lasterror = "Corrupt secondary header.";
+                                               ret = VCEDIT_ERR_INVAL;
                                                goto err;
                                        }
 
@@ -383,7 +374,7 @@ vcedit_open (vcedit_state *state)
                bytes = fread (buffer, 1, CHUNKSIZE, state->in);
 
                if (bytes == 0 && i < 2) {
-                       state->lasterror = "EOF before end of vorbis headers.";
+                       ret = VCEDIT_ERR_INVAL;
                        goto err;
                }
 
@@ -394,15 +385,17 @@ vcedit_open (vcedit_state *state)
        state->vendor = strdup (state->vc->vendor);
 
        /* Headers are done! */
-       return 0;
+       state->opened = true;
+
+       return VCEDIT_ERR_SUCCESS;
 
 err:
        vcedit_clear_internals (state);
 
-       return -1;
+       return ret;
 }
 
-int
+vcedit_error
 vcedit_write (vcedit_state *state)
 {
        ogg_stream_state streamout;
@@ -414,23 +407,22 @@ vcedit_write (vcedit_state *state)
        int s, result, bytes, needflush = 0, needout = 0;
        size_t tmp;
 
+       if (!state->opened)
+               return VCEDIT_ERR_INVAL;
+
        strcpy (tmpfile, state->filename);
        strcat (tmpfile, ".XXXXXX");
 
        s = mkstemp (tmpfile);
-       if (s == -1) {
-               state->lasterror = "Error writing stream to output. "
-                                  "Cannot open temporary file.";
-               return -1;
-       }
+       if (s == -1)
+               return VCEDIT_ERR_TMPFILE;
 
        out = fdopen (s, "wb");
        if (!out) {
                unlink (tmpfile);
                close (s);
-               state->lasterror = "Error writing stream to output. "
-                                  "Cannot open temporary file.";
-               return -1;
+
+               return VCEDIT_ERR_TMPFILE;
        }
 
        state->prevW = state->extrapage = state->eosin = 0;
@@ -548,9 +540,7 @@ vcedit_write (vcedit_state *state)
                        if (!result)
                 break;
 
-                       if (result < 0)
-                               state->lasterror = "Corrupt or missing data, continuing...";
-                       else {
+                       if (result >= 0) {
                                /* Don't bother going through the rest, we can just
                                 * write the page out now
                                 */
@@ -593,14 +583,11 @@ cleanup:
 
     state->mainbuf = state->bookbuf = NULL;
 
-       if (!state->eosin) {
-               state->lasterror = "Error writing stream to output. "
-                                  "Output stream may be corrupted or truncated.";
-               return -1;
-       }
+       if (!state->eosin)
+               return VCEDIT_ERR_INVAL;
 
        vcedit_clear_internals (state);
-       vcedit_open (state);
 
-       return 0;
+       return (vcedit_open (state) == VCEDIT_ERR_SUCCESS) ?
+              VCEDIT_ERR_SUCCESS : VCEDIT_ERR_REOPEN;
 }

diff --git a/ext/vcedit.h b/ext/vcedit.h
index cdd7be513c2bca55786b7e9a5362284d6d948cf3..c0f15dccb9a64f1b12bbd91112d0678e311e74e5 100644 (file)
--- a/ext/vcedit.h
+++ b/ext/vcedit.h
@@ -27,15 +27,22 @@ extern "C" {
 #include <ogg/ogg.h>
 #include <vorbis/codec.h>
 
+typedef enum {
+       VCEDIT_ERR_SUCCESS = 0,
+       VCEDIT_ERR_OPEN,
+       VCEDIT_ERR_INVAL,
+       VCEDIT_ERR_TMPFILE,
+       VCEDIT_ERR_REOPEN
+} vcedit_error;
+
 typedef struct vcedit_state_St vcedit_state;
 
 vcedit_state *vcedit_state_new (const char *filename);
 void vcedit_state_ref (vcedit_state *state);
 void vcedit_state_unref (vcedit_state *state);
 vorbis_comment *vcedit_comments (vcedit_state *state);
-int vcedit_open (vcedit_state *state);
-int vcedit_write (vcedit_state *state);
-const char *vcedit_error (vcedit_state *state);
+vcedit_error vcedit_open (vcedit_state *state);
+vcedit_error vcedit_write (vcedit_state *state);
 
 #ifdef __cplusplus
 }