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