On Thu, Oct 10, 2013 at 09:37:32AM +0100, David Laight wrote: > > Since we removed the __packed from most of the packets, we should > > make sure that the offset generated by the compiler are correct for > > sent/received data. > ... > > + /* compile time checks for struct member offsets */ > > + BUILD_BUG_ON(offsetof(struct batadv_unicast_4addr_packet, src) != 10); > > + BUILD_BUG_ON(offsetof(struct batadv_unicast_packet, dest) != 4); > > + BUILD_BUG_ON(offsetof(struct batadv_unicast_frag_packet, dest) != 4); > > + BUILD_BUG_ON(offsetof(struct batadv_unicast_tvlv_packet, dst) != 4); > > + BUILD_BUG_ON(offsetof(struct batadv_icmp_packet, dst) != 4); > > + BUILD_BUG_ON(offsetof(struct batadv_icmp_packet_rr, dst) != 4); > > It is usually enough to check the size of the structures.
What if two fields are inverted by mistake in a way that the size of the struct remains the same? The size check would not complain but the code would not work anymore. We use a "generic" struct to access the initial part of any packet. Therefore these checks are to ensure that the information we are going to access is really placed at that offset, whatever packet we have. It was not possible to use a common inner struct and so we relied on this test to be safe. > Which is also best done in the .h file so it is validated > in all the compilation environments that might be used. > This does not really hurt at the moment because we placed them in main.c which is a file that is always compiled. But thanks for the suggestion: putting them in the .h file helps to remind the developer to add a new BUILD_ON_BUG when creating a new packet type. Thanks a lot. Regards, -- Antonio Quartulli
signature.asc
Description: Digital signature
