"Gilbert Ramirez Jr." wrote:
>
> typedef struct _e_igmp {
> #if BYTE_ORDER == BIG_ENDIAN
> guint8 igmp_v:4;
> guint8 igmp_t:4;
> #else /* Little endian */
> guint8 igmp_t:4;
> guint8 igmp_v:4;
> #endif
> guint8 igmp_unused;
> guint16 igmp_cksum;
> guint32 igmp_gaddr;
> } e_igmp;
>
> We see the nibbles being re-arranged in the e_igmp struct in an attempt
> to overcome the non-portability (by checking the endianness of the
> CPU), but the ability to use bitfields inside an unsigned char is
> itself non-portable. We should probably not rely on gcc's ability to
> address bits inside unsigned chars.
That's right.
> Furthermore, I don't think ethereal should use the structs we see in
> packet.h as templates where you do a memcpy of the entire structure
> from the packet data to your structure. When using a compiler other
> than gcc, you cannot guarantee that your fields will not be padded to
> fit in 4-byte boudnaries. So, for example, one cannot guarantee that
> igmp_unused and igmp_cksum are next to each other, or are 3 bytes apart.
>
> Unless I'm wrong, we should copy each field of the struct from the packet
> data into the struct, instead of memcpy'ing the entire struct at once. As
> an example, see packet-tr.c. I copy each field, one by one.
I think this is not a good solution. memcpy'ing the whole struct is
preferable to copying each field. We have just to correctly define a
particular structure. For instance, if you define the igmp structure in
such a way:
{
guint_8 igmp_type;
guint_8 igmp_code;
guint_16 igmp_cksum;
guint_32 igmp_gaddr;
}
I don't know any modern C compiler that will do the wrong thing about
alignment. We have just to define structures in a compiler-independent
way and to handle any further endianness problem in the code itself rather
than in the structure definition.
Laurent.
--
Laurent DENIEL | E-mail: deniel@xxxxxxxxxxx
Paris, FRANCE | deniel@xxxxxxxxxxxxxxxxxxxxxxxxxxxx
| WWW : http://www.worldnet.fr/~deniel
All above opinions are personal, unless stated otherwise.
