This is me <[EMAIL PROTECTED]>, no access to the other mail from here.
Curt McDowell wrote:
struct ip_addr2 is only used as a hack in etharp.c and should be removed.
etharp_arp_input() should use simple memcpy()s to copy the struct ip_addr,
because that code isn't in a critical path.
memcpy(&sipaddr, &hdr->sipaddr, sizeof(sipaddr));
memcpy(&dipaddr, &hdr->dipaddr, sizeof(dipaddr));
The following comment in etharp_arp_input() is wrong:
/* these are aligned properly, whereas the ARP header fields might not be */
struct ip_addr sipaddr, dipaddr;
Because struct ip_addr is packed, sipaddr and dipaddr have an alignment
requirement of 1. This booby trap results in really erratic behavior, say if
the "u8_t i" is moved up. I was using struct ip_addr in my code and this
problem cost me a few hours. struct ip_addr should not be packed. Only the
packet format structures that contain it need to be packed.
Right!
Bet you were casting from u32_t to an unaligned struct ip_addr, no?
Seems like the right thing to do.
If the packing is removed from struct ip_addr, and struct ip_addr2 is
removed, then there is no need to memcpy anymore.
A simple:
sipaddr = hdr->sipaddr;
will do.
And this should also trigger some optimizations that weren't possible because
of the packing,
saving a few (not many) .text bytes.
I can only look at this again next week, I will provide a patch then if noone
does it before.
Cheers,
Pedro Alves
Regards,
Curt McDowell
Broadcom Corp.
-----Original Message-----
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On
Behalf Of Pedro Alves
Sent: Friday, April 21, 2006 3:22 AM
To: Mailing list for lwIP users
Subject: Re: [lwip-users] [PATCH] fix warning for gcc and
possible unaligned access
Timmy Brolin wrote:
Pedro Alves wrote:
[EMAIL PROTECTED] wrote:
Hi,
Why not just replace the lines
*(struct ip_addr2 *)&sipaddr = hdr->sipaddr;
*(struct ip_addr2 *)&dipaddr = hdr->dipaddr; with
sipaddr = *(struct ip_addr *)&hdr->sipaddr;
dipaddr = *(struct ip_addr *)&hdr->dipaddr;
Because if hdr->sipaddr is unaligned and the architecture doesn't
support unaligned accesses, like many RISCs do, the result
is undefined.
Are you sure that it can become unaligned?
There is code in place in lwip to guarantee header alignment...
Sure they can, that's why the copying is beeing made.
I missed the fact that ip_addr and ip_addr2 are both pack_struct'ed.
The first time I looked I got the impression that struct
ip_addr2 was packed and struct ip_addr wasn't.
Beach's suggestion removes the gcc warning and is used
throughout lwip, but is it really safe?
-*(struct ip_addr2 *)&sipaddr = hdr->sipaddr; -*(struct
ip_addr2 *)&dipaddr = hdr->dipaddr;
+sipaddr = *(struct ip_addr *)&hdr->sipaddr; dipaddr =
*(struct ip_addr
+*)&hdr->dipaddr;
The only safe way I've heard that ensures correct conversion
is using unions like this:
/* These tell the
* compiler that the type punning between the two pointer
classes needs to
* be taken into account when optimizing.
*/
#define IP_ADDR_TO_IP_ADDR2(FROM, TO) \
do { \
union ip_add_pun { \
struct ip_addr ip; \
struct ip_addr2 ip2; \
} d; \
d.ip = (FROM); \
(TO) = d.ip2; \
} while (0)
#define IP_ADDR2_TO_IP_ADDR(FROM, TO) \
do { \
union ip_add_pun { \
struct ip_addr ip; \
struct ip_addr2 ip2; \
} d; \
d.ip2 = (FROM); \
(TO) = d.ip; \
} while (0)
IP_ADDR2_TO_IP_ADDR(hdr->sipaddr, sipaddr);
IP_ADDR2_TO_IP_ADDR(hdr->dipaddr, dipaddr);
Can anyone comment on why we need struct ip_addr and struct ip_addr2?
Cheers,
Pedro Alves
_______________________________________________
lwip-users mailing list
[email protected]
http://lists.nongnu.org/mailman/listinfo/lwip-users
_______________________________________________
lwip-users mailing list
[email protected]
http://lists.nongnu.org/mailman/listinfo/lwip-users
_______________________________________________
lwip-users mailing list
[email protected]
http://lists.nongnu.org/mailman/listinfo/lwip-users
