This line:
printf ("tm.b = 0x%x\n", (unsigned long)tm.b);
Would most likely cause a crash.
The problem is that if the address of tm.a is for example 0x1000, then
the address of tm.b will be 0x1001.
If you try to perform a 32bit read or write operation on tm.b then it
will crash if the CPU does not support unaligned accesses.
If you leave out the packed attribute, then the compiler will insert
three dummy bytes between tm.a and tm.b to solve the unaligned access
problem. This is of course not an option in the lwip case since we use
structs to describe packet headers.
Regards,
Timmy Brolin
Pedro Alves wrote:
Timmy Brolin wrote:
Curt McDowell wrote:
Pedro Alves wrote
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.
memcpy may still be needed because hdr->sipaddr is misaligned at +2
due to the
format of the ARP packet. My theory is that struct ip_addr2 was
originally
invented because the 4-byte struct/uint32 copy was crashing on
someone's 2-byte
aligned CPU!
Regards,
Curt McDowell
Broadcom Corp.
Either ip_addr2 or memcpy is needed.
The code "sipaddr = hdr->sipaddr;" would crash on any CPU which do
not support unaligned accesses.
huh? Not if struct ip_addr is NOT packed, hdr is an instance of a
packed struct, and sipaddr is a struct ip_addr, not struct ip_addr2.
Are you saying that:
#include <stdio.h>
struct test_me
{
unsigned char a;
unsigned long b;
unsigned char c;
} __attribute__ ((packed));
int main(int argc, char** argv)
{
volatile char dummy_offset_alignment_for_next_var;
volatile struct test_me tm;
volatile unsigned long i;
volatile unsigned long i2;
tm.a = 0xdd;
tm.c = 0xee;
i = 0x55ffaa33;
tm.b = i;
i2 = tm.b;
printf("sizeof(struct test_me) = 0x%x\n", sizeof(struct test_me));
printf ("tm.a = 0x%x\n", (unsigned long)tm.a);
printf ("tm.b = 0x%x\n", (unsigned long)tm.b);
printf ("tm.c = 0x%x\n", (unsigned long)tm.c);
printf ("i2 = 0x%x\n", i2);
return 0;
}
Won't work as expected? That would beat the whole purpose of
__attribute__((packed)), no?
P.S.: No use testing in x86, as that supports unaligned accesses anyway.
Cheers,
Pedro Alves
The current ip_addr2 solution is technically more efficient than
memcpy since it copies the ip_addr with just two 16bit reads+writes.
The only solution which would be slightly more efficient is to copy
hdr->sipaddr to a 32bit register using two 16bit accesses, and then
write the 32bit register to sipaddr. But since this is not a critical
code path, it is more important to keep the code as clean as possible
rather than trying to save one single memory access.
The ip_addr2 solution makes the code nice and clean.
Regards,
Timmy Brolin
_______________________________________________
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
