Marek Lindner wrote:
On Saturday 28 May 2011 00:20:21 Sven Eckelmann wrote:
struct unicast_frag_packet {
uint8_t packet_type; uint8_t version; /* batman version field */
uint8_t dest[6]; uint8_t ttl;
uint8_t align;uint8_t dest[6]; uint8_t flags;uint8_t align; uint8_t orig[6]; uint16_t seqno;} __packed;
Wait a second - are there two "align" in a single struct. Sry, have to Nack that one :)
Yes, I suggested that because the tt patches will replace the first align with the ttvn field.
But it doesn't compile. I cannot forward stuff which is too broken. And personally I don't care who suggested it when my builtin c-parser explodes.
Ok, now I did the "work" to apply and and to compile that stuff to prove my point. Here are the slightly shortened results (gcc-4.6 from sid):
CC [M] aggregation.o In file included from types.h:27:0, from main.h:121, from aggregation.c:22: packet.h:114:11: error: duplicate member ‘align’ make[2]: *** [aggregation.o] Error 1 make[1]: *** [_module_/batman-adv] Error 2 make[1]: Leaving directory `linux-2.6.39' make: *** [all] Error 2
My suggestion: Rename the first align in every packet structure to 'reserved'.
Then to the next part which I don't like: the patch forgot to touch batman_packet, icmp_packet, icmp_packet_rr, bcast_packet and vis_packet. It doesn't make sense to patch half of the on-wire types to reduce the number of COMPAT_VERSIONs.
Kind regards, Sven