Re: [lwip-users] [PATCH] fix warning for gcc and possible unaligned access

[Pedro Alves]

Timmy Brolin wrote:
> Reading through the mailing list archive from two years ago refreshed 
> my memory a bit.
> For the Texas DSP+compiler the problem was that it do not support 
> packed structs.
> Several people had the same or similar problems related to unaligned 
> fields in the structs.
>
> The 04/27/04 fix of bug #8708 solves all this by doing two things:
> A 2 byte pad is added at the very beginning of the packet. This makes 
> all the header fields problerly aligned, with one exception: The IP 
> fields in the ARP header.
> This is why ip_addr2 is there. By splitting the 32bit ip_addr into two 
> 16bit fields all fields are properly aligned so that the packed struct 
> attribute is no longer required for most compilers.
> This solved the problem for everyone back then.
> The 2 byte pad should actually improve performance a bit as well since 
> it makes all the IP and TCP headers nice and aligned.
>
> I have to agree with you that the code must be fixed to comply with C 
> standards. I guess memcpy is fine.
> But replacing ip_addr2 with ip_addr will break things on all compilers 
> that do not support packed structs. Possibly others as well.
>
> If your compiler generates special code to handle potential alignment 
> problems whenever packed structs are used even if there are no 
> alignment issues, then I suggest you try disabling packed structs. 
> lwip in its current state should compile fine on most compilers even 
> without packed structs.

I went through the archives and read that discussion. I have better 
understanding of struct ip_addr2 now. Thanks for the pointer.
I say, go with memcpy then, and be done with it. The type punning was my 
original concern.
If struct ip_addr2 is kept, is there still a reason to have struct 
ip_addr packed for compilers that do support packing?

Cheers,
Pedro Alves
> Regards,
> Timmy Brolin
>
> Curt McDowell wrote:
>
> > Timmy Brolin wrote:
> >  
> >
> > > There is a very good reason why ip_addr2 was introduced. ip_addr2 is 
> > > in fact a decent solution. It is generates close optimal machine 
> > > code, it is not compiler dependent, and it makes the code nice and 
> > > easy to read.
> > > I would suggest that it remains unless someone comes up with a 
> > > better solution which does not break compatibility with any compilers.
> > >   
> >
> > I disagree that ip_addr2 is in any way elegant, self-explanatory, 
> > easy to read
> > (being defined elsewhere but not used elsewhere), or that it necessarily
> > compiles to two half-word transfers, or that performance is remotely 
> > critical
> > here.  Worse, it's not acceptable code in C99.  People are changing 
> > it to
> > trivial memcpy because:
> >
> > etharp.c:493: warning: dereferencing type-punned pointer will break
> > strict-aliasing rules
> > *(struct ip_addr2 *)&sipaddr = hdr->sipaddr;
> >
> > Regards,
> > Curt McDowell
> > Broadcom Corp.