[lwip-devel] LWIP_U32_DIFF is unnecessarily complex

[Mason]

Hello,

src/include/lwip/def.h defines LWIP_U32_DIFF
a macro which used to compute the difference between two
u32_t values, taking wrap-around into account.

/** Get the absolute difference between 2 u32_t values (correcting overflows)
 * 'a' is expected to be 'higher' (without overflow) than 'b'. */
#define LWIP_U32_DIFF(a, b) (((a) >= (b)) ? ((a) - (b)) : (((a) + ((b) ^ 
0xFFFFFFFF) + 1)))

Given a 32-bit unsigned value, C guarantees that the
expression ((b) ^ 0xFFFFFFFF) + 1 is always equal to
the expression -b.

Let b' = b ^ 0xFFFFFFFF then b + b' = 0xFFFFFFFF
b + b' + 1 = 0 (mod 2^32)
b' + 1 = -b (mod 2^32)

You can also run the following program:

#include <stdio.h>
int main(void)
{
  volatile unsigned b = 0;
  do
  {
    unsigned v1 = (b ^ 0xFFFFFFFF) + 1;
    unsigned v2 = -b;
    if (v1 != v2) printf("b=%u v1=%u v2=%u\n", b, v1, v2);
  }
  while (++b != 0);
  return 0;
}

(The volatile qualifier prevents some compilers from
optimizing the entire loop away.)

Therefore LWIP_U32_DIFF reduces to

#define LWIP_U32_DIFF(a, b) ((a)-(b))

I suggest removing the macro altogether, and applying the
following patch.

--- timers.c.orig       2011-05-06 10:51:25.000000000 +0200
+++ timers.c    2011-08-24 14:22:08.218750000 +0200
@@ -365,7 +365,7 @@
   now = sys_now();
   if (next_timeout) {
     /* this cares for wraparounds */
-    diff = LWIP_U32_DIFF(now, timeouts_last_time);
+    diff = now - timeouts_last_time;
     do
     {
       had_one = 0;

-- 
Regards.