[MPlayer-cvslog] CVS: main/libmpdemux ebml.c,1.4,1.5

[Uoti A Urpala]

On Sun, Jul 10, 2005 at 08:31:15PM +0200, Moritz Bunkus CVS wrote:
> Fix for gcc 4 and strict-aliasing. Patch by Uoti A Urpala ( urpala () cc ! helsinki ! fi ).

Reimar Döffinger wrote:
> I don't really agree with this patch.
> 1) this by far isn't the only place where we break strict-aliaing
> 2) It makes the code more complicated (which is also true for that
> (float *) (void *) that was there before)
> 3) from what I read somewhere else, this code code _still_ does not
> behave according to strict aliasing rules, you would have to assing the
> pointer to a void* variable first.

According to the gcc man page accessing the same memory as different
types is safe if either one of the types is a character type or the
accesses are through different members of the same union. So the patch
should work correctly, no additional void* casts required. The original
code had a (void *) cast, which did no good but instead prevented gcc
from giving the (appropriate) warning about potential strict-aliasing
problems.

I think changing the long double case in my patch was not necessary,
as there the write was done using uint8_t (which presumably qualifies
as a "character type"). However the other two cases (write uint32_t /
read float, write uint64_t / read double) needed to be changed.

I looked at the places where gcc currently gives strict-aliasing
warnings for mplayer. Most of them seemed to be casts in function
calls and not likely to cause problems (though I'm not 100% sure).
There was one place that was clearly dangerous and likely to create
broken code: libfaad2/ic_predict.c, function flt_round which modifies
variables as uint32_t, then reads them as float32_t.