[lwip-devel] [patch #6521] lwip doesn't compile in 64_bit computers

[Jonathan Larmour]

Follow-up Comment #1, patch #6521 (project lwip):

Thanks for the patch.

I see what you are trying to do, but a global search and replace of %d with
%"S_32F" is not right. You say they should be u32_t or s32_t but that's not
true. We don't need to lay down a size for most of these, and for many of the
user-supplied data with the sockets interface (such as socket descriptors) we
must stay compatible with the well-known standard types of the BSD interface.
Arguments declared as int should stay with %d in debug output, and so on for
similar arguments. It is only arguments of u16_t or s16_t etc. which should be
changed if they need to be.

Also this change in lwip_select:
-  LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_select(%d, %p, %p, %p, tvsec=%ld
tvusec=%ld)n",
+  LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_select(%"S32_F", %p, %p, %p, tvsec=%lu
tvusec=%lu)n",
                   maxfdp1, (void *)readset, (void *) writeset, (void *)
exceptset,
-                  timeout ? timeout->tv_sec : -1L, timeout ?
timeout->tv_usec : -1L));
+                  timeout ? timeout->tv_sec : -1, timeout ? timeout->tv_usec
: -1));

The second change won't be right (although what was there before wasn't
perfect either). Telling it -1 is an unsigned value won't be good. And on some
platforms where sizeof(int) != sizeof(long) and struct timeval's tv fields
aren't longs, the stack could get mangled. It should probably instead stay
with %ld format specifiers and use this for args:
 timeout ? (long)timeout->tv_sec : -1L, timeout ? (long)timeout->tv_usec :
-1L)
This allows it to adapt somewhat to systems which set LWIP_TIMEVAL_PRIVATE to
0.

Your changes to _IOR and _IOW aren't really the right answer, although long
wasn't necessarily ideal either: they should probably be u32_t casts to
explicitly guarantee the bitfield is large enough (consider targets with
16-bit ints).

If you could rework the patch to address these issues, that would be great,
thanks!


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6521>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/