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

[Pedro Alves]

Timmy Brolin wrote:

> Pedro Alves wrote:
>
> > address@hidden 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