[FFmpeg-devel] [PATCH 2/3] avformat/vividas: Check allocation for success

[Andreas Rheinhardt]

Zane van Iperen:
> On Thu, 6 Aug 2020 01:33:57 +0200
> "Andreas Rheinhardt" <andreas.rheinhardt at gmail.com> wrote:
> 
>>
>>  static void load_sb_block(AVFormatContext *s, VividasDemuxContext *viv, unsigned expected_size)
>> @@ -608,7 +606,7 @@ static int viv_read_header(AVFormatContext *s)
>>      ret = track_index(viv, s, buf, v);
>>      av_free(buf);
>>      if (ret < 0)
>> -        return ret;
>> +        goto fail;
>>
>>      viv->sb_offset = avio_tell(pb);
>>      if (viv->n_sb_blocks > 0) {
>> @@ -619,6 +617,9 @@ static int viv_read_header(AVFormatContext *s)
>>      }
>>
>>      return 0;
>> +fail:
>> +    av_freep(&viv->sb_blocks);
>> +    return ret;
>>  }
> 
> Nit: Considering there's only one `goto fail`, wouldn't it be better to
> just av_freep and return immediately instead?
> 
This patch is designed so that this demuxer can easily be converted to
automatically call read_close on read_header failure (see [1] for
details, in fact all bugs in this patchset have been found when working
on this goal (I already have 61 demuxers now (17 more to go in
libavformat plus all the demuxers in libavdevice)). Doing cleanup the
way I do it here implies that the patch that marks this demuxer as
compatible with automatic freeing can simply remove the whole fail part
above and replace "goto fail" with return ret again. But if I cleaned up
in place, I would have to touch the "if (ret < 0)" line now and either
unnecessarily touch it again when the cleanup will be made automatic or
keep the unnecessary {} in "if (ret < 0) {\n return ret;\n}".

- Andreas