[FFmpeg-cvslog] r21029 - trunk/libavcodec/alsdec.c

[Thilo Borgmann]

Am 05.01.10 23:16, schrieb Reimar D?ffinger:
> On Tue, Jan 05, 2010 at 02:04:37PM +0100, Thilo Borgmann wrote:
>> Am 05.01.10 09:02, schrieb Uoti Urpala:
>>> On Tue, 2010-01-05 at 08:39 +0100, Reimar D?ffinger wrote:
>>>> On Tue, Jan 05, 2010 at 08:19:28AM +0200, Uoti Urpala wrote:
>>>>> My guess is that the actual cause of the crash was wrapping arithmetic
>>>>> in the original subtraction ("smp" is unsigned). The new version changes
>>>
>>>> Then the question is: why in the world is smp unsigned, and why is the
>>>> code obfuscated instead of giving it a more appropriate type?
>>>> Looking at the code I can't see even remotely any reason why smp should
>>>> be unsigned.
>>>
>>> Some people think it's a good idea to make variables unsigned just
>>> because they're not supposed to hold negative values. Usually they're
>>> not aware how it causes exactly this kind of problems. This has been
>>> discussed before, for example
>>> http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/2008-July/015559.html
>>
>> Uoti seems to be right. Also, using a signed int smp solves the issue by
>> still using [] to dereference.
>>
>> So a) use working pointer math like it is now,
>> or b) switch back and use "int smp",
>> or c) let's wait until I've played around with Michaels optimization
>> suggestion (will be today) and see if we still need to discuss this.
> 
> Just put it on your TODO that this code is bad readability-wise and fix it when
> it's convenient (within a reasonable time scale).

Finally fixed with revision 21069 by avoiding smp in index arithmetic.

-Thilo