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

[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).