[PATCH] Fix unaligned read (was:Re: [MPlayer-dev-eng] Solaris patches)

[Guillaume POIRIER]

Hi,

On 9/5/06, Robin KAY <komadori at gekkou.co.uk> wrote:
> Diego Biurrun wrote:
>
> > Seconded, make it one patch per mail, I have completely lost track a
> > long time ago.
>
> Fix access to potentially unaligned fields in the ASF header.
>
> --
> Wishing you good fortune,
> Robin KAY (komadori)
>
>
> diff -ru mplayer-head-20060902-orig/libmpdemux/asfheader.c mplayer-head-20060902/libmpdemux/asfheader.c
> --- mplayer-head-20060902-orig/libmpdemux/asfheader.c   Sat Sep  2 19:18:24 2006
> +++ mplayer-head-20060902/libmpdemux/asfheader.c        Sat Sep  2 21:30:20 2006
> @@ -406,7 +406,8 @@
>          uint32_t max_bitrate;
>          char *ptr = &hdr[pos];
>          mp_msg(MSGT_HEADER,MSGL_V,"============ ASF Stream group == START ===\n");
> -        stream_count = le2me_16(*(uint16_t*)ptr);
> +        memcpy(&stream_count, ptr, sizeof(uint16_t)); // align value
> +        stream_count = le2me_16(stream_count);
>          ptr += sizeof(uint16_t);
>          if (ptr > &hdr[hdr_len]) goto len_err_out;
>          if(stream_count > 0)
> @@ -413,7 +414,8 @@
>                streams = malloc(2*stream_count*sizeof(uint32_t));
>          mp_msg(MSGT_HEADER,MSGL_V," stream count=[0x%x][%u]\n", stream_count, stream_count );
>          for( i=0 ; i<stream_count ; i++ ) {
> -          stream_id = le2me_16(*(uint16_t*)ptr);
> +          memcpy(&stream_id, ptr, sizeof(uint16_t)); // align value
> +          stream_id = le2me_16(stream_count);
>            ptr += sizeof(uint16_t);
>            if (ptr > &hdr[hdr_len]) goto len_err_out;
>            memcpy(&max_bitrate, ptr, sizeof(uint32_t));// workaround unaligment bug on sparc

Isn't there any other way to fix read alignement issues on Sparc other
then by indroducing a memcopy?

Can't this be solved by using the macro DECLARE_ALIGNED if the
variable was allocated on the stack or by making sure malloc aligns
memory properly dynamicly allocated memory?

I'm afraid that your solution looks more like a hack than the proper
solution, but I may be wrong. It also probably slows down execution on
all archs (because of the unnecessary copy) just to solve a problems
with a platform that has a very small userbase.

I'd say patch rejected unless you can convince me that there's no other way.

Guillaume
-- 
With DADVSI (http://en.wikipedia.org/wiki/DADVSI), France finally has
a lead on USA on selling out individuals right to corporations!
Vive la France!