Re: [B.A.T.M.A.N.] [PATCH] batman-adv: move the ttl field to the third position of unicast_* packets

Sven Eckelmann
2011-05-28T07:26:30Z

[-- Attachment #1: Type: Text/Plain, Size: 1742 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]