[FFmpeg-cvslog] r9733 - trunk/libavformat/os_support.c
Benoit Fouet
benoit.fouet
Thu Jul 19 11:39:22 CEST 2007
Michael Niedermayer wrote:
> Hi
>
> On Thu, Jul 19, 2007 at 10:52:38AM +0200, Benoit Fouet wrote:
>
>> Uoti Urpala wrote:
>>
>>> On Thu, 2007-07-19 at 10:17 +0200, Benoit Fouet wrote:
>>>
>>>
>>>> M?ns Rullg?rd wrote:
>>>>
>>>>
>>>>>> add1 = atoi(pch);
>>>>>> pch = strpbrk(pch,".");
>>>>>> - if (pch == 0 || ++pch == 0) return 0;
>>>>>> + if (!pch) return 0;
>>>>>> add2 = atoi(pch);
>>>>>>
>>>>>>
>>>
>>>
>>>>> This is wrong. The ++ is needed. Whoever wrote this code was very
>>>>> confused.
>>>>>
>>>>>
>>>>>
>>>> then the right thing to do would be:
>>>> if(!pch) return 0;
>>>> addX = atoi(++pch);
>>>>
>>>> right ? tome, the check of ++pch is really unneeded
>>>>
>>>>
>>> The ++ is needed to move over the '.', but yes check against 0 will
>>> always be false. The next character of the string (there must be one if
>>> the string is 0-terminated and the current character is not 0) will not
>>> be at address 0.
>>>
>>> I guess it was meant to be a check against the pointed-to character
>>> being the terminating 0. However even with that the error checking in
>>> that function would be really weak - it'll accept addresses such as
>>> "1.12foo.bar.something.32000.1.1.2" (I think that would be parsed the
>>> same as "1.12.0.0").
>>>
>>>
>>>
>> so what about using something like
>> if( sscanf(pch, "%d.%d.%d.%d", &add1, &add2, &add3, &add4) != 4 )
>> return 0;
>>
>> instead of the strpbrk stuff ?
>>
>
> fine with me
>
> [...]
>
>
see attached patch, i also removed the pch definition
--
Ben
Purple Labs S.A.
www.purplelabs.com
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: os_support.c.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20070719/b3a2abf8/attachment.asc>
More information about the ffmpeg-cvslog
mailing list