mp4: Improve parse_tag().

* Merge tag_add_field() and read_string() into parse_tag() since they
are simple enough and have only one caller.

* Avoid memory leaks in the error case.

* Let the function return an error code (rather than -1) in all cases,
and check the return value in the callers.

* Add a sanity check for the subsize.

* Avoid creating two copies of the tag value.

* Rename the variable for the tag value.

diff --git a/mp4.c b/mp4.c
index 479a2b69c562b7c0724e68032259dc4c2330f908..02dd5a190ff519179b5b94aabe7ae66bcf55cb91 100644 (file)
--- a/mp4.c
+++ b/mp4.c
@@ -620,39 +620,6 @@ static int read_stsd(struct mp4 *f)
        return 1;
 }
 
-static int32_t tag_add_field(struct mp4_metadata *meta, const char *item,
-               const char *value, int32_t len)
-{
-       meta->tags = para_realloc(meta->tags,
-               (meta->count + 1) * sizeof(struct mp4_tag));
-       meta->tags[meta->count].item = para_strdup(item);
-       meta->tags[meta->count].len = len;
-       if (len >= 0) {
-               meta->tags[meta->count].value = para_malloc(len + 1);
-               memcpy(meta->tags[meta->count].value, value, len);
-               meta->tags[meta->count].value[len] = 0;
-       } else {
-               meta->tags[meta->count].value = para_strdup(value);
-       }
-       meta->count++;
-       return 1;
-}
-
-static int read_string(struct mp4 *f, uint32_t length, char **result)
-{
-       char *str = para_malloc(length + 1);
-       int ret = read_data(f, str, length);
-
-       if (ret <= 0) {
-               free(str);
-               *result = NULL;
-       } else {
-               str[length] = '\0';
-               *result = str;
-       }
-       return ret;
-}
-
 static const char *get_metadata_name(uint8_t atom_type)
 {
        switch (atom_type) {
@@ -669,9 +636,10 @@ static int parse_tag(struct mp4 *f, uint8_t parent, int32_t size)
 {
        int ret;
        uint64_t subsize, sumsize;
-       char *data = NULL;
+       char *value = NULL;
        uint32_t len = 0;
        uint64_t destpos;
+       struct mp4_tag *tag;
 
        for (
                sumsize = 0;
@@ -682,30 +650,43 @@ static int parse_tag(struct mp4 *f, uint8_t parent, int32_t size)
                uint8_t header_size = 0;
                ret = atom_read_header(f, &atom_type, &header_size, &subsize);
                if (ret <= 0)
-                       return ret;
+                       goto fail;
                destpos = get_position(f) + subsize - header_size;
                if (atom_type != ATOM_DATA)
                        continue;
                ret = read_int8(f, NULL); /* version */
                if (ret <= 0)
-                       return ret;
+                       goto fail;
                ret = read_int24(f, NULL); /* flags */
                if (ret <= 0)
-                       return ret;
+                       goto fail;
                ret = read_int32(f, NULL); /* reserved */
                if (ret <= 0)
-                       return ret;
-               free(data);
-               ret = read_string(f, subsize - (header_size + 8), &data);
-               if (ret <= 0)
-                       return ret;
+                       goto fail;
+               ret = -ERRNO_TO_PARA_ERROR(EINVAL);
+               if (subsize < header_size + 8 || subsize > UINT_MAX)
+                       goto fail;
                len = subsize - (header_size + 8);
+               free(value);
+               value = para_malloc(len + 1);
+               ret = read_data(f, value, len);
+               if (ret <= 0)
+                       goto fail;
+               value[len] = '\0';
        }
-       if (!data)
-               return -1;
-       tag_add_field(&f->meta, get_metadata_name(parent), data, len);
-       free(data);
+       if (!value)
+               return -ERRNO_TO_PARA_ERROR(EINVAL);
+       f->meta.tags = para_realloc(f->meta.tags, (f->meta.count + 1)
+               * sizeof(struct mp4_tag));
+       tag = f->meta.tags + f->meta.count;
+       tag->item = para_strdup(get_metadata_name(parent));
+       tag->value = value;
+       tag->len = len;
+       f->meta.count++;
        return 1;
+fail:
+       free(value);
+       return ret;
 }
 
 static int read_mdhd(struct mp4 *f)
@@ -780,7 +761,9 @@ static int32_t read_ilst(struct mp4 *f, int32_t size)
                case ATOM_ALBUM:
                case ATOM_COMMENT:
                case ATOM_DATE:
-                       parse_tag(f, atom_type, subsize - header_size);
+                       ret = parse_tag(f, atom_type, subsize - header_size);
+                       if (ret <= 0)
+                               return ret;
                }
                set_position(f, destpos);
                sumsize += subsize;
@@ -807,9 +790,11 @@ static int32_t read_meta(struct mp4 *f, uint64_t size)
                        return ret;
                if (subsize <= header_size + 4)
                        return 1;
-               if (atom_type == ATOM_ILST)
-                       read_ilst(f, subsize - (header_size + 4));
-               else
+               if (atom_type == ATOM_ILST) {
+                       ret = read_ilst(f, subsize - (header_size + 4));
+                       if (ret <= 0)
+                               return ret;
+               } else
                        set_position(f, get_position(f) + subsize - header_size);
                sumsize += subsize;
        }