diff mbox series

[FFmpeg-devel,1/3] avformat/mxfdec: Fix memleak when adding element to array fails

Message ID 20200720061545.18854-1-andreas.rheinhardt@gmail.com
State Accepted
Commit 49e78548c35be84200ea9f617c4b5b2f58c7e6f6
Headers show
Series [FFmpeg-devel,1/3] avformat/mxfdec: Fix memleak when adding element to array fails | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt July 20, 2020, 6:15 a.m. UTC
Said array contains pointers to other structs and both the designated
new element as well as other stuff contained in it (e.g. strings) leak
if the new element can't be added to the array.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/mxfdec.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Tomas Härdin July 22, 2020, 12:35 p.m. UTC | #1
mån 2020-07-20 klockan 08:15 +0200 skrev Andreas Rheinhardt:
> Said array contains pointers to other structs and both the designated
> new element as well as other stuff contained in it (e.g. strings)
> leak
> if the new element can't be added to the array.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/mxfdec.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index 90546d42b3..08ad92cc0c 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -822,15 +822,17 @@ static int mxf_read_partition_pack(void *arg,
> AVIOContext *pb, int tag, int size
>      return 0;
>  }
>  
> -static int mxf_add_metadata_set(MXFContext *mxf, void *metadata_set)
> +static int mxf_add_metadata_set(MXFContext *mxf, MXFMetadataSet
> **metadata_set)
>  {
>      MXFMetadataSet **tmp;
>  
>      tmp = av_realloc_array(mxf->metadata_sets, mxf-
> >metadata_sets_count + 1, sizeof(*mxf->metadata_sets));
> -    if (!tmp)
> +    if (!tmp) {
> +        mxf_free_metadataset(metadata_set, 1);

Went and double-checked that mxf_free_metadataset nulls *metadata_set
which it does, so this patch OK.

/Tomas
diff mbox series

Patch

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 90546d42b3..08ad92cc0c 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -822,15 +822,17 @@  static int mxf_read_partition_pack(void *arg, AVIOContext *pb, int tag, int size
     return 0;
 }
 
-static int mxf_add_metadata_set(MXFContext *mxf, void *metadata_set)
+static int mxf_add_metadata_set(MXFContext *mxf, MXFMetadataSet **metadata_set)
 {
     MXFMetadataSet **tmp;
 
     tmp = av_realloc_array(mxf->metadata_sets, mxf->metadata_sets_count + 1, sizeof(*mxf->metadata_sets));
-    if (!tmp)
+    if (!tmp) {
+        mxf_free_metadataset(metadata_set, 1);
         return AVERROR(ENOMEM);
+    }
     mxf->metadata_sets = tmp;
-    mxf->metadata_sets[mxf->metadata_sets_count] = metadata_set;
+    mxf->metadata_sets[mxf->metadata_sets_count] = *metadata_set;
     mxf->metadata_sets_count++;
     return 0;
 }
@@ -2780,7 +2782,7 @@  static int mxf_read_local_tags(MXFContext *mxf, KLVPacket *klv, MXFMetadataReadF
             avio_seek(pb, next, SEEK_SET);
     }
     if (ctx_size) ctx->type = type;
-    return ctx_size ? mxf_add_metadata_set(mxf, ctx) : 0;
+    return ctx_size ? mxf_add_metadata_set(mxf, &ctx) : 0;
 }
 
 /**
@@ -3083,10 +3085,8 @@  static int mxf_handle_missing_index_segment(MXFContext *mxf, AVStream *st)
     if (!(segment = av_mallocz(sizeof(*segment))))
         return AVERROR(ENOMEM);
 
-    if ((ret = mxf_add_metadata_set(mxf, segment))) {
-        mxf_free_metadataset((MXFMetadataSet**)&segment, 1);
+    if ((ret = mxf_add_metadata_set(mxf, (MXFMetadataSet**)&segment)))
         return ret;
-    }
 
     /* Make sure we have nonzero unique index_sid, body_sid will be ok, because
      * using the same SID for index is forbidden in MXF. */