From 7f29c8effd9e1e534176d453b37e507e0600d7e7 Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Sat, 14 Aug 2021 22:27:12 +0200 Subject: [PATCH] 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. --- mp4.c | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/mp4.c b/mp4.c index 7f223bc3..7085e959 100644 --- 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); -- 2.39.5