mp4: Check return value of membuffer_transfer_from_file().

This function calls the ->read() method of the callback, which may
fail. Currently all three callers ignore the return value and rely
on the fact that the membuffer is set to error state, which will be
detected later.

It's easier and clearer to check for errors in the callers and fail
early on read errors. Since the membuffer is useless in the error
case, free it right away in membuffer_transfer_from_file(). Change
the function to return bool instead of unsigned while at it and remove
a pointless cast in one of its callers.

diff --git a/mp4.c b/mp4.c
index 7f223bc3ea67853bd2b6e42ee49eeb5f9c3949fe..7085e95918b626eb917269c3389db8bb77161130 100644 (file)
--- a/mp4.c
+++ b/mp4.c
@@ -1296,12 +1296,7 @@ static void *membuffer_get_ptr(const struct membuffer *buf)
        return buf->data;
 }
 
-static void membuffer_set_error(struct membuffer *buf)
-{
-       buf->error = 1;
-}
-
-static unsigned membuffer_transfer_from_file(struct membuffer *buf, struct mp4 *src,
+static bool membuffer_transfer_from_file(struct membuffer *buf, struct mp4 *src,
                unsigned bytes)
 {
        unsigned oldsize;
@@ -1309,19 +1304,18 @@ static unsigned membuffer_transfer_from_file(struct membuffer *buf, struct mp4 *
 
        oldsize = membuffer_get_size(buf);
        if (membuffer_write(buf, 0, bytes) != bytes)
-               return 0;
+               return false;
 
        bufptr = membuffer_get_ptr(buf);
        if (bufptr == 0)
-               return 0;
+               return false;
 
        if ((unsigned)read_data(src, (char *) bufptr + oldsize, bytes) !=
                bytes) {
-               membuffer_set_error(buf);
-               return 0;
+               membuffer_free(buf);
+               return false;
        }
-
-       return bytes;
+       return true;
 }
 
 static uint32_t create_meta(const struct mp4_metadata *meta, void **out_buffer,
@@ -1396,8 +1390,10 @@ static void *modify_moov(struct mp4 *f, uint32_t *out_size)
 
                buf = membuffer_create();
                set_position(f, total_base);
-               membuffer_transfer_from_file(buf, f, total_size);
-
+               if (!membuffer_transfer_from_file(buf, f, total_size)) {
+                       free(new_udta_buffer);
+                       return NULL;
+               }
                membuffer_write_atom(buf, "udta", new_udta_size,
                        new_udta_buffer);
 
@@ -1420,12 +1416,18 @@ static void *modify_moov(struct mp4 *f, uint32_t *out_size)
 
                buf = membuffer_create();
                set_position(f, total_base);
-               membuffer_transfer_from_file(buf, f,
-                       (uint32_t)(udta_offset - total_base));
+               if (!membuffer_transfer_from_file(buf, f,
+                               udta_offset - total_base)) {
+                       free(new_meta_buffer);
+                       return NULL;
+               }
 
                membuffer_write_int32(buf, udta_size + 8 + new_meta_size);
                membuffer_write_atom_name(buf, "udta");
-               membuffer_transfer_from_file(buf, f, udta_size);
+               if (!membuffer_transfer_from_file(buf, f, udta_size)) {
+                       free(new_meta_buffer);
+                       return NULL;
+               }
 
                membuffer_write_atom(buf, "meta", new_meta_size,
                        new_meta_buffer);