[FFmpeg-devel] remove int readers

[Måns Rullgård]

ods15 at ods15.dyndns.org wrote:
> On Thu, Jun 14, 2007 at 11:07:12AM +0200, Baptiste Coudurier wrote:
>> Hi guys,
>>
>> Michael Niedermayer wrote:
>> > Hi
>> >
>> > On Thu, Jun 14, 2007 at 09:59:32AM +0200, Reimar Doeffinger wrote:
>> >> Hello,
>> >> On Thu, Jun 14, 2007 at 08:21:08AM +0100, M??ns Rullg??rd wrote:
>> >>> "Ronald S. Bultje" <rsbultje at gmail.com> writes:
>> >>>> On 6/14/07, mark cox <melbournemark+ffmpeg at gmail.com> wrote:
>> >>>>> -    seq  = (buf[2] << 8) | buf[3];
>> >>>>> +    seq  = AV_RB16(buf + 2);
>> >>>>>
>> >>>>> Why have you changed all the indexes from [] to +. [] are preferred
>> >>>>> because they avoid compiler warnings and are easier to read IMHO.
>> >>>> Consistency with the rest of the code. I prefer &buf[2] also, but I try
>> to
>> >>>> conform to what's already there.
>> >>> &buf[2] is IMO far harder to read than buf + 2, and it's ugly to
>> >>> boot.
>> >> I don't think we will ever get a common opinion on this, I usually prefer
>> >> &buf[2] syntax and find buf + 2 ugly ;-)
>> >
>> > i am on mans side this time :)
>> > also +2 is 2 chars &[2] is 4 so its simpler ;)
>> >
>> > anyway, we should be carefull to seperating such bikeshed issues from real
>> > clear readability issues, i dont want ffmpeg to become known for rejecting
>> > patches because of such style issues ...
>> >
>>
>> Im with Mans and Michael, sorry Reimar ;)
>
> I'm not on any side, but would like to note a really bad bug I had in tcc
> once when trying to compile ffmpeg, because of this issue:
>
> int8_t buf[4] = {5,1,0,0};
> int a = *(int32_t*)(buf + 0); // gets value 261
> int b = *(int32_t*)&buf[0];   // gets value 5 :(
>
> I had to chase this down all the way in dsputil.c, by seperating it to 2
> files, one compiled by gcc and one by tcc, moving functions around one by
> one... :)

Well, that looks like a compiler bug.  Those two expressions should be
equivalent.  That said, this is probably violating some aliasing rules
so all bets are off.

-- 
M?ns Rullg?rd
mans at mansr.com