On Sat, 15 Jan 2011, Bruce Evans wrote:
...
Later code in tcp_output uses bogus casts to long and larger code instead:
% if (recwin < (long)(tp->rcv_adv - tp->rcv_nxt))
% recwin = (long)(tp->rcv_adv - tp->rcv_nxt);
% if (recwin > (long)TCP_MAXWIN << tp->rcv_scale)
% recwin = (long)TCP_MAXWIN << tp->rcv_scale;
% ...
% if (recwin > 0 && SEQ_GT(tp->rcv_nxt + recwin, tp->rcv_adv))
% tp->rcv_adv = tp->rcv_nxt + recwin;
Note that the first statement avoids using the technically incorrect
SEQ_FOO() although its internals are better (cast to int instead of
long). It uses cases essentially like yours. Then further analysis
^^^^^ a cast
is simpler because everything is converted to long. The second starement
is similar to the first half of the broken expression. Large code using
if's and else's and tests (x >= y) before subtracting y from x is much
easier to get right than 1 complicated 1-statement expression like the
broken one. It takes these (x >= y) tests to make code with mixed types
obviously correct. But I prefer small fast code with ints for everything,
since type analyis is too hard.
But the casts to long are not good. Here they have no effect except
to break the warning about the bad type of `recwin'. recwin has type
long, so assignment to it does the same conversion as the cast, possibly
with a warning about the implicit conversion if it might overflow (can
overflow only on 32-bit arches). The code depends on rcv_adv being
sequentially >= rcv_next with or without the cast. Otherwise, the
difference is huge unsigned int, and the cast only changes this (by
benign overflow) on 32-bit arches.
This can be fixed by casting to int instead of long (now the cast may have
an effect, and breaking the warning may be intential), or my proposed
SEQ_DIFF() macros work well here:
int recwin, sd;
...
sd = SEQ_NONNEG_NONLARGE_DIFF(tp->rcv_adv, tp->rcv_nxt);
/*
* SEQ_DIFF() supports negative differences;
* SEQ_NONNEG_NONGLARGE_DIFF() KASSERT()s that they don't happen and
* are not too large. This name is too long.
*/
if (recwin < sd)
recwin = sd;
...
Bruce
_______________________________________________
freebsd-bugs@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "freebsd-bugs-unsubscr...@freebsd.org"
