On Thu, Aug 20, 2020 at 12:46:12PM +0200, Andreas Rheinhardt wrote: > Michael Niedermayer: > > On Wed, Aug 19, 2020 at 12:00:37AM +0200, Andreas Rheinhardt wrote: > >> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> > >> --- > >> The memleak can be reproduced with e.g. the first 163 bytes of > >> https://samples.ffmpeg.org/archive/all/avi+dvvideo+pcm_s16le++ffmpeg-avidec554-crash.avi > >> > >> libavformat/avidec.c | 31 +++++++++++++++++-------------- > >> 1 file changed, 17 insertions(+), 14 deletions(-) > >> > >> diff --git a/libavformat/avidec.c b/libavformat/avidec.c > >> index 5fc3e01aa9..08b864f19a 100644 > >> --- a/libavformat/avidec.c > >> +++ b/libavformat/avidec.c > >> @@ -113,6 +113,7 @@ static const AVMetadataConv avi_metadata_conv[] = { > >> { 0 }, > >> }; > >> > >> +static int avi_read_close(AVFormatContext *s); > >> static int avi_load_index(AVFormatContext *s); > >> static int guess_ni_flag(AVFormatContext *s); > >> > >> @@ -464,6 +465,7 @@ static int calculate_bitrate(AVFormatContext *s) > >> return 1; > >> } > >> > >> +#define RETURN_ERROR(code) do { ret = (code); goto fail; } while (0) > >> static int avi_read_header(AVFormatContext *s) > >> { > >> AVIContext *avi = s->priv_data; > >> @@ -499,7 +501,7 @@ static int avi_read_header(AVFormatContext *s) > >> frame_period = 0; > >> for (;;) { > >> if (avio_feof(pb)) > >> - goto fail; > >> + RETURN_ERROR(AVERROR_INVALIDDATA); > > > > this macro is messy > > it replaces writing > > {ret = AVERROR_INVALIDDATA; goto fail;} > > by > > RETURN_ERROR(AVERROR_INVALIDDATA); > > > > this is almost the same length but the first is directly understood C code > > the 2nd is harder to understand for someone reading the code so i > > suggest to avoid the 2nd and use something else, not saying that needs to > > be the first > > > The only reason this macro exists is because it allows me to add code > that can easily be removed lateron when cleaning up after read_header > failure will be automatic, whereas an > > if (foo) { > ret = bar; > goto fail; > } > > leads to a bigger diff now and later. If you want to, I could of course use
I have no oppinion about the intermediate state but i think we should have optimally readable/clean code in the end after all planned changes thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB "Nothing to hide" only works if the folks in power share the values of you and everyone you know entirely and always will -- Tom Scott
signature.asc
Description: PGP signature
_______________________________________________ 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".
