Hacker News new | ask | show | jobs
by cube13 5570 days ago
It's a working fix, but isn't the proper fix. The real issue is the size difference of the long type for 32 and 64-bit architectures. Bruce Evans doesn't explicitly mention this fact, but it's the core of his reasoning.

Since long is 64-bits on 64-bit architecture, and 32 on 32-bit architecture. This is the reason that 0xffffffff is showing up as a non-negative number on the 64-bit machine, but shows up as negative on the 32-bit.

Changing the type to int(which is 32 bits long on both x86 and x64), while it does break 16-bit systems(which don't exist anymore), fixes this completely by removing the x86 and x64 behavior differences with the long type.

The two style issues that he mentions are easily fixed by moving the variable declaration to the top of the function and initializing it there. However, these may be forced by the function structure due to the gotos present in the code...

1 comments

a better fix to use the lmin macro. libkern.h.

http://www.freebsd.org/cgi/cvsweb.cgi/~checkout~/src/sys/sys...

Not really. The line is this: long adv = min(recwin, (long)TCP_MAXWIN << tp->rcv_scale) - (tp->rcv_adv - tp->rcv_nxt);

So it's really: long = uint(long, (long)uint32) - (uint32-uint32)

Here's the problem on x64: you're converting a 64-bit long to a uint, then doing a subtraction with another uint, then placing that result into a 64-bit long. Since the compiler is just doing an assignment rather than a sign extension, which is why there is a large positive number rather than -1.

Changing it to use the lmin macro would make it: long = long(long, (long)uint32) - (uint32-uint32)

This still has the underlying issue(using 32-bit values in 64-bit buckets), which should work out fine, but may have issues down the road.

It makes more sense to change all the long types to int32/uint32 types rather than just cast longs everywhere. If recwin and adv were changed to int32, it would be: int32 = uint32(int32, uint32) - (uint32 - uint32)

While this potentially has issues if the uints are between 0x80000000 and 0xfffffff, it's a safer solution than using longs.

EDIT: added some explanation

the lmin[1] macro will convert the uints to a long int and then do the arithmetic.

[1]static __inline long lmin(long a, long b) { return (a < b ? a : b); }

[2]static __inline u_int min(u_int a, u_int b) { return (a < b ? a : b); }

Edit: 6.3.1.8 Usualarithmetic conversions in the c-99 standard.