[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[lwip-devel] [patch #6521] lwip doesn't compile in 64_bit computers
From: |
Jonathan Larmour |
Subject: |
[lwip-devel] [patch #6521] lwip doesn't compile in 64_bit computers |
Date: |
Mon, 26 May 2008 01:06:21 +0000 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.13) Gecko/20060513 Fedora/1.0.8-1.1.fc3.1.legacy Firefox/1.0.8 |
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/