Zane van Iperen: > On Thu, 6 Aug 2020 01:33:57 +0200 > "Andreas Rheinhardt" <andreas.rheinha...@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".