[FFmpeg-devel] [PATCH] Use avio_read instead of get_strz to read mov string.

Reimar Döffinger Reimar.Doeffinger
Mon Mar 7 22:29:24 CET 2011


On 7 Mar 2011, at 17:01, "Ronald S. Bultje" <rsbultje at gmail.com> wrote:
> On Sun, Mar 6, 2011 at 8:08 AM, Reimar D?ffinger
> <Reimar.Doeffinger at gmx.de> wrote:
>> This makes more sense because we have a length of a string,
>> so it works better in case 0-termination is missing or corrupted,
>> matches other string read functions in mov.c and also adds a check
>> that avoids an integer overflow (even though it does not cause issues
>> due to signed types used properly).
>> ---
>>  libavformat/mov.c |    4 +++-
>>  1 files changed, 3 insertions(+), 1 deletions(-)
>> 
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index be799a4..1d32f31 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -2331,7 +2331,9 @@ static void mov_read_chapters(AVFormatContext *s)
>>             avio_get_str16le(sc->pb, len, title, title_len);
>>         else {
>>             AV_WB16(title, ch);
>> -            get_strz(sc->pb, title + 2, len - 1);
>> +            if (len > 2)
>> +                avio_read(sc->pb, title + 2, len - 2);
>> +            title[len] = 0;
> 
> This code now uses avio_get_str(), does that always properly take care
> of NULL-terminating? (It should, looking at the code.)

That code is even more obfuscated, and currently behaves different from all other string reading functions in this file (including the two other cases right here) insofar as it will cut the last byte if the string is not 0-terminated.
I do not know if that behaviour is wrong, but it is inconsistent and thus bad.



More information about the ffmpeg-devel mailing list