[tor-dev] [Stegotorus] Strange comparison in chop_circuit_t::send_targeted(conn, blocksize)

[Zack Weinberg]

On 2012-08-10 8:11 AM, vmon wrote:
> Hey Zack,
>
> Please take a look at this comparison in
> chop_circuit_t::send_targeted(conn, blocksize):
>
>    if (avail > blocksize - lo)
>      avail = blocksize - lo;
>    else if (avail > SECTION_LEN)
>      avail = SECTION_LEN;
>    else if (upstream_eof && !sent_fin)
>      // this block will carry the last byte of real data to be sent in
>      // this direction; mark it as such
>      op = op_FIN;
>
> Assume SECTION_LEN = 65536 suppose
> avail = 67000 blocksize - lo = 66500 => avail = 66500
> avail = 67000 blocksize - lo = 67500 => avail = 65536
>
> it's like that if avail is very big then the it is OK if it's bigger than
> SECTION_LEN but if it's moderately big then it shouldn't be bigger than
> SECTION_LEN, which seems very arbitrary.

The *intent* here is to clamp 'avail' to
min(blocksize - lo, SECTION_LEN) because: we can't fit more than 
SECTION_LEN bytes of data in a block under any circumstances, and we 
were asked to provide a block which is no larger than 'blocksize' but we 
have 'lo' bytes of overhead to cope with.

So your rewrite is correct.  I think maybe the control flow here could 
do with some going-over for clarity but that needn't happen right now.

>    if ((avail > blocksize - lo) || (avail > SECTION_LEN))

The nested parentheses are unnecessary in this case.

>    {
>      avail = min(blocksize - lo, SECTION_LEN);
>    }

No curly braces around one-statement blocks.

zw