[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Timmy Brolin
Subject: Re: [lwip-users] [PATCH] fix warning for gcc and possible unaligned access
Date: Tue, 25 Apr 2006 22:46:59 +0200
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4) Gecko/20030624 Netscape/7.1 (ax)

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.

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;

Curt McDowell
Broadcom Corp.

lwip-users mailing list

reply via email to

[Prev in Thread] Current Thread [Next in Thread]