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

[Sven Eckelmann <sven.eckelmann@gmx.de>]

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

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: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]