mp4: Implement error checking for the write path.

Although the ->write callback has a return value, it is of unsigned
type and is never checked. Fix this by changing the prototype to
match that of the write(2) system call, check the return value of the
callback in the write_data() wrapper of mp4.c and propagate paraslash
error codes back to aac_afh.c via the public mp4_meta_update().

While at it, handle short writes and EINTR properly, and fix the
indentation of the callback structure in mp4.h.

diff --git a/aac_afh.c b/aac_afh.c
index f3a06c4c213c80fad95158d8c33b18fafa43e33a..0a80bfcf3a44205ea7d42f9ac260c03f910ba199 100644 (file)
--- a/aac_afh.c
+++ b/aac_afh.c
@@ -173,10 +173,10 @@ static uint32_t aac_afh_meta_seek_cb(void *user_data, uint64_t pos)
        return lseek(fd, pos, SEEK_SET);
 }
 
-static uint32_t aac_afh_meta_write_cb(void *user_data, void *dest, uint32_t want)
+static ssize_t aac_afh_meta_write_cb(void *user_data, void *dest, size_t count)
 {
        int fd = *(int *)user_data;
-       return write(fd, dest, want);
+       return write(fd, dest, count);
 }
 
 static uint32_t aac_afh_meta_truncate_cb(void *user_data)
@@ -238,11 +238,7 @@ static int aac_afh_rewrite_tags(const char *map, size_t mapsize,
        replace_or_add_tag("album", tags->album, metadata);
        replace_or_add_tag("date", tags->year, metadata);
        replace_or_add_tag("comment", tags->comment, metadata);
-       ret = -E_MP4_META_WRITE;
-       if (!mp4_meta_update(mp4))
-               goto close;
-       ret = 1;
-close:
+       ret = mp4_meta_update(mp4);
        mp4_close(mp4);
        return ret;
 }

diff --git a/error.h b/error.h
index 671d0612a972ed777b55b30cadf5fd18dbd07ce6..843c4ee8c23c0b8a32e7867981cfaa222b8fac0a 100644 (file)
--- a/error.h
+++ b/error.h
@@ -141,7 +141,6 @@
        PARA_ERROR(MP4_BAD_SAMPLE, "mp4: invalid sample number"), \
        PARA_ERROR(MP4_BAD_SAMPLERATE, "mp4: invalid sample rate"), \
        PARA_ERROR(MP4_BAD_SAMPLE_COUNT, "mp4: invalid number of samples"), \
-       PARA_ERROR(MP4_META_WRITE, "mp4: could not update mp4 metadata"), \
        PARA_ERROR(MP4_OPEN, "mp4: open failed"), \
        PARA_ERROR(MP4_TRACK, "mp4: no audio track"), \
        PARA_ERROR(MP4_MISSING_ATOM, "mp4: essential atom not found"), \

diff --git a/mp4.c b/mp4.c
index 535c785aa1c62443697d8f0c78fabb29759270c0..fc8c41b4af919358350812cae68033611c5d33c4 100644 (file)
--- a/mp4.c
+++ b/mp4.c
@@ -970,22 +970,27 @@ static void *modify_moov(struct mp4 *f, uint32_t *out_size)
        return out_buffer;
 }
 
-static int32_t write_data(struct mp4 *f, void *data, uint32_t size)
+static int write_data(struct mp4 *f, void *data, size_t size)
 {
-       int32_t result = 1;
-
-       result = f->cb->write(f->cb->user_data, data, size);
-
-       f->current_position += size;
-
-       return result;
+       while (size > 0) {
+               ssize_t ret = f->cb->write(f->cb->user_data, data, size);
+               if (ret < 0) {
+                       if (errno == EINTR)
+                               continue;
+                       return -ERRNO_TO_PARA_ERROR(errno);
+               }
+               f->current_position += ret;
+               size -= ret;
+       }
+       return 1;
 }
 
-int32_t mp4_meta_update(struct mp4 *f)
+int mp4_meta_update(struct mp4 *f)
 {
        void *new_moov_data;
        uint32_t new_moov_size;
-       uint8_t buf[4];
+       uint8_t buf[8] = "----moov";
+       int ret;
 
        set_position(f, 0);
        new_moov_data = modify_moov(f, &new_moov_size);
@@ -995,17 +1000,24 @@ int32_t mp4_meta_update(struct mp4 *f)
        }
        if (f->last_atom != ATOM_MOOV) {
                set_position(f, f->moov_offset + 4);
-               write_data(f, "free", 4); /* rename old moov to free */
+               ret = write_data(f, "free", 4); /* rename old moov to free */
+               if (ret < 0)
+                       goto free_moov;
                set_position(f, f->file_size); /* write new moov atom at EOF */
        } else /* overwrite old moov atom */
                set_position(f, f->moov_offset);
        write_u32_be(buf, new_moov_size + 8);
-       write_data(f, buf, 4);
-       write_data(f, "moov", 4);
-       write_data(f, new_moov_data, new_moov_size);
-       free(new_moov_data);
+       ret = write_data(f, buf, sizeof(buf));
+       if (ret < 0)
+               goto free_moov;
+       ret = write_data(f, new_moov_data, new_moov_size);
+       if (ret < 0)
+               goto free_moov;
        f->cb->truncate(f->cb->user_data);
-       return 1;
+       ret = 1;
+free_moov:
+       free(new_moov_data);
+       return ret;
 }
 
 static char *meta_find_by_name(const struct mp4 *f, const char *item)

diff --git a/mp4.h b/mp4.h
index 783de19d631d4a09bb3c364d96451003eb253d00..30a609f38c108d0306ccc18ea68596cf64647c96 100644 (file)
--- a/mp4.h
+++ b/mp4.h
@@ -1,9 +1,9 @@
 struct mp4_callback {
-    ssize_t (*read)(void *user_data, void *buffer, size_t length);
-    uint32_t (*write)(void *udata, void *buffer, uint32_t length);
-    uint32_t (*seek)(void *user_data, uint64_t position);
-    uint32_t (*truncate)(void *user_data);
-    void *user_data;
+       ssize_t (*read)(void *user_data, void *buffer, size_t length);
+       ssize_t (*write)(void *user_data, void *buffer, size_t count);
+       uint32_t (*seek)(void *user_data, uint64_t position);
+       uint32_t (*truncate)(void *user_data);
+       void *user_data;
 };
 
 struct mp4_tag {
@@ -29,7 +29,7 @@ int32_t mp4_num_samples(const struct mp4 *f);
 uint64_t mp4_get_duration(const struct mp4 *f);
 int mp4_open_meta(const struct mp4_callback *cb, struct mp4 **result);
 struct mp4_metadata *mp4_get_meta(struct mp4 *f);
-int32_t mp4_meta_update(struct mp4 *f);
+int mp4_meta_update(struct mp4 *f);
 char *mp4_meta_get_artist(const struct mp4 *f);
 char *mp4_meta_get_title(const struct mp4 *f);
 char *mp4_meta_get_date(const struct mp4 *f);