On Tue, Feb 15, 2011 at 06:12:17PM +0100, Linus L??ssing wrote:
Just a minor style adjustment, to give people a hint which fields should not be reordered and need to be at the beginning of each packet with batman-adv's frame type.
Hi Linus
+struct batman_header {
- uint8_t packet_type;
- uint8_t version; /* batman version field */
- uint8_t ttl;
- uint8_t align;
+} __packed;
struct batman_packet {
- uint8_t packet_type;
- uint8_t version; /* batman version field */
- struct batman_header header;
- uint32_t seqno; uint8_t flags; /* 0x40: DIRECTLINK flag, 0x20 VIS_SERVER flag... */ uint8_t tq;
- uint32_t seqno; uint8_t orig[6]; uint8_t prev_sender[6];
- uint8_t ttl; uint8_t num_hna; uint8_t gw_flags; /* flags related to gateway class */
- uint8_t align;
} __packed;
Two different ideas, both triggered by the align byte in header. This byte is a waste of space, it is not used, and it is probably not easy to find something which all packet types are going to need in the future.
1) Did you try not having it. So long header is __packed, and all the structures it is used in are __packed, i think the compiler will do the right thing. The downside is that alignment is not obvious. Most people will assume header is 4 bytes, not 3, and think the alignment of the packets is wrong.
2) Did you consider using a union? It is a more invasive patch, but the alignment is then clear. The downside is sizeof() no longer gives you the per packet type size, rather it gives you the size of the biggest member of the union. So this makes the change even more invasive and bug prone.
I think i prefer 1), with appropriate comments to explain the alignment issue.
Andrew