[FFmpeg-devel] [PATCH 2/3] pmpdec: fix signedness

[Don Moir]

----- Original Message ----- 
From: "Reimar Döffinger" <Reimar.Doeffinger at gmx.de>
To: "FFmpeg development discussions and patches" <ffmpeg-devel at ffmpeg.org>
Sent: Saturday, February 23, 2013 5:46 PM
Subject: Re: [FFmpeg-devel] [PATCH 2/3] pmpdec: fix signedness


>
>
> On 23 Feb 2013, at 22:08, Michael Niedermayer <michaelni at gmx.at> wrote:
>
>> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
>> ---
>> libavformat/pmpdec.c |    4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavformat/pmpdec.c b/libavformat/pmpdec.c
>> index 358f7b6..38eba14 100644
>> --- a/libavformat/pmpdec.c
>> +++ b/libavformat/pmpdec.c
>> @@ -44,7 +44,7 @@ static int pmp_header(AVFormatContext *s)
>>     PMPContext *pmp = s->priv_data;
>>     AVIOContext *pb = s->pb;
>>     int tb_num, tb_den;
>> -    int index_cnt;
>> +    unsigned index_cnt;
>>     int audio_codec_id = AV_CODEC_ID_NONE;
>>     int srate, channels;
>>     int i;
>> @@ -93,7 +93,7 @@ static int pmp_header(AVFormatContext *s)
>>     channels = avio_rl32(pb) + 1;
>>     pos = avio_tell(pb) + 4*index_cnt;
>>     for (i = 0; i < index_cnt; i++) {
>> -        int size = avio_rl32(pb);
>> +        unsigned size = avio_rl32(pb);
>
> Why not go all the way and use uint32_t ?
> Seems safest to do.
> All patches look ok to me.

By changing size and index_cnt to uint32_t it introduces 3 warnings for signed/unsigned mismatch for me when compiling that code in 
Visual Studio. You don't see that in mingw. But more than that, it seems to me by keeping them as int you get an automatic check for 
an out of bounds condition. That is, an exceesively large number will be negative and fail accordingly.