On Mon, Nov 16, 2015 at 8:12 AM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote:
> On Mon, Nov 16, 2015 at 7:53 AM, Hendrik Leppkes <h.lepp...@gmail.com> wrote:
>> On Mon, Nov 16, 2015 at 1:52 PM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote:
>>> On Mon, Nov 16, 2015 at 3:27 AM, wm4 <nfx...@gmail.com> wrote:
>>>> On Sun, 15 Nov 2015 17:56:22 -0500
>>>> Ganesh Ajjanagadde <gajjanaga...@gmail.com> wrote:
>>>>
>>>>> ffio_ensure_seekback can fail due to e.g ENOMEM. This return value is
>>>>> propagated here, and all usage in the codebase now has its return value
>>>>> checked.
>>>>>
>>>>> A potential memory leak in mp3_read_header is also fixed via a goto
>>>>> fail.
>>>>>
>>>>> Signed-off-by: Ganesh Ajjanagadde <gajjanaga...@gmail.com>
>>>>> ---
>>>>> libavformat/mp3dec.c | 12 +++++++++---
>>>>> libavformat/rmdec.c | 3 ++-
>>>>> 2 files changed, 11 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
>>>>> index 32ca00c..9fefe2d 100644
>>>>> --- a/libavformat/mp3dec.c
>>>>> +++ b/libavformat/mp3dec.c
>>>>> @@ -373,18 +373,20 @@ static int mp3_read_header(AVFormatContext *s)
>>>>>
>>>>> ret = ff_replaygain_export(st, s->metadata);
>>>>> if (ret < 0)
>>>>> - return ret;
>>>>> + goto fail;
>>>>>
>>>>> off = avio_tell(s->pb);
>>>>> for (i = 0; i < 64 * 1024; i++) {
>>>>> uint32_t header, header2;
>>>>> int frame_size;
>>>>> if (!(i&1023))
>>>>> - ffio_ensure_seekback(s->pb, i + 1024 + 4);
>>>>> + if ((ret = ffio_ensure_seekback(s->pb, i + 1024 + 4)) < 0)
>>>>> + goto fail;
>>>>> frame_size = check(s->pb, off + i, &header);
>>>>> if (frame_size > 0) {
>>>>> avio_seek(s->pb, off, SEEK_SET);
>>>>> - ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 4);
>>>>> + if ((ret = ffio_ensure_seekback(s->pb, i + 1024 + frame_size
>>>>> + 4)) < 0)
>>>>> + goto fail;
>>>>> if (check(s->pb, off + i + frame_size, &header2) >= 0 &&
>>>>> (header & SAME_HEADER_MASK) == (header2 &
>>>>> SAME_HEADER_MASK))
>>>>> {
>>>>> @@ -402,6 +404,10 @@ static int mp3_read_header(AVFormatContext *s)
>>>>>
>>>>> /* the parameters will be extracted from the compressed bitstream */
>>>>> return 0;
>>>>> +
>>>>> +fail:
>>>>> + ff_free_stream(s, st);
>>>>> + return ret;
>>>>> }
>>>>>
>>>>> #define MP3_PACKET_SIZE 1024
>>>>> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
>>>>> index 4ec78ef..d6e820e 100644
>>>>> --- a/libavformat/rmdec.c
>>>>> +++ b/libavformat/rmdec.c
>>>>> @@ -576,7 +576,8 @@ static int rm_read_header(AVFormatContext *s)
>>>>> size = avio_rb32(pb);
>>>>> codec_pos = avio_tell(pb);
>>>>>
>>>>> - ffio_ensure_seekback(pb, 4);
>>>>> + if ((ret = ffio_ensure_seekback(pb, 4)) < 0)
>>>>> + goto fail;
>>>>> v = avio_rb32(pb);
>>>>> if (v == MKBETAG('M', 'L', 'T', 'I')) {
>>>>> int number_of_streams = avio_rb16(pb);
>>>>
>>>> NACK. There's no reason to fatally fail in these cases.
>>>
>>> Ok, will split into two for the memory leak and these return values.
>>> For the return values, will simply log at AV_LOG_WARNING.
>>>
>>
>> There is no actual memory leak here, the stream is free'ed when the
>> format context is closed.
>
> Thanks for clarifying, indeed it is part of the context via
> avformat_new_stream. I was concerned that this was being called
> multiple times. Seems like it is all ok, and checked the docs.
addressed and reposted.
>
>>
>> - Hendrik
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
