Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Fixing wrap-around bug in vis

[Simon Wunderlich <simon.wunderlich@s2003.tu-chemnitz.de>]

[-- Attachment #1: Type: text/plain, Size: 1963 bytes --]

Hey,

nice catch!

I'd like to join the party and propose this version:

#define seq_before(x,y) ((int8_t) (x - y) < 0)
#define seq_after(x,y) seq_before(y,x)

Not so much bitshifting and may a little bit easier to 
understand, but also not general and does not pass Svens regression test ... :(

Wrong seq_before(0, 128) == true
Wrong seq_after(0, 128) == true
Wrong seq_before(1, 129) == true
Wrong seq_after(1, 129) == true
Wrong seq_before(2, 130) == true
Wrong seq_after(2, 130) == true
Wrong seq_before(3, 131) == true
Wrong seq_after(3, 131) == true
Wrong seq_before(4, 132) == true
Wrong seq_after(4, 132) == true
Wrong seq_before(5, 133) == true
Wrong seq_after(5, 133) == true

What do you think?  ;)

best regards
	Simon

On Thu, Mar 11, 2010 at 11:06:42PM +0100, Sven Eckelmann wrote:
> Linus Lüssing wrote:
> > -               if (vis_packet->seqno - old_info->packet.seqno <= 0) {
> > +               if (vis_packet->seqno - old_info->packet.seqno
> > +                   <= 1 << 7 * sizeof(vis_packet->seqno)) {
> 
> Shouldn't that be 
>  1 << 7 + 8 * (sizeof(vis_packet->seqno) - 1))?
> 
> Otherwise you would have left the the sizeof(vis_packet->seqno) most 
> significant bits 0 and the rest one. And maybe a seq_before/seq_after static 
> inline function could be created to make this thing a lot more readable. You 
> could also do that in a macro if you don't want to assume the type of 
> vis_packet->seqno... which is probably what you tried by using sizeof
> 
> #define seq_before(x,y) ((x - y) >= 1 << 7 + 8 * (sizeof(x) - 1))
> #define seq_after(x,y) ((y - x) >= 1 << 7 + 8 * (sizeof(x) - 1))
> ....
> 
> And maybe you see here that the equal part which is needed bellow that line 
> isn't possible. So it must be changed to 
> 
> if (!seq_after(vis_packet->seqno, old_info->packet.seqno))
> 
> I hope that this is right - please check twice.
> 
> 
> Best regards,
> 	Sven



[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]