Alexandre,
I am afraid the following patch
revision 1.52 date: 2003/03/31 23:56:35; author: julliard; state: Exp; lines: +63 -60 Try to make winsock.h more portable (based on a patch by Francois Gouget).
actually caused a portability problem for FreeBSD.
/usr/bin/gcc -c -I. -I. -I../../../include -I../../../include -g -O2 -Wall -mpreferred-stack-boundary=2 -gstabs+ -Wpointer-arith -fPIC -D_REENTRANT -o pipe.o pipe.c In file included from ../../../include/winsock2.h:47, from ../../../include/windows.h:62, from ../../../include/rpc.h:22, from ../../../include/wtypes.h:2, from pipe.c:34: ../../../include/winsock.h:925: conflicting types for `htonl' /usr/include/machine/endian.h:63: previous declaration of `htonl' ../../../include/winsock.h:926: conflicting types for `htons' /usr/include/machine/endian.h:64: previous declaration of `htons' ../../../include/winsock.h:927: conflicting types for `ntohl' /usr/include/machine/endian.h:65: previous declaration of `ntohl' ../../../include/winsock.h:928: conflicting types for `ntohs' /usr/include/machine/endian.h:66: previous declaration of `ntohs' gmake[3]: *** [pipe.o] Error 1 gmake[3]: Leaving directory `/.amd_mnt/nashira/files5/test/wine/dlls/kernel/tests' gmake[2]: *** [tests] Error 2 gmake[2]: Leaving directory `/.amd_mnt/nashira/files5/test/wine/dlls/kernel'
The FreeBSD declarations are unsigned long htonl __P((unsigned long)); unsigned short htons __P((unsigned short)); unsigned long ntohl __P((unsigned long)); unsigned short ntohs __P((unsigned short));
Sorry, Gerald
Gerald Pfeifer pfeifer@dbai.tuwien.ac.at writes:
I am afraid the following patch
revision 1.52 date: 2003/03/31 23:56:35; author: julliard; state: Exp; lines: +63 -60 Try to make winsock.h more portable (based on a patch by Francois Gouget).
actually caused a portability problem for FreeBSD.
I changed the way we handle htonl a bit. Does it work better now?
On Wed, 1 Apr 2003, Alexandre Julliard wrote:
I changed the way we handle htonl a bit. Does it work better now?
Yes, thanks a lot! Now it builds, though I'm getting warnings:
In file included from /usr/include/sys/types.h:126, from ../../include/winsock.h:89, from ../../include/winsock2.h:47, from ../../include/windows.h:62, from icinfo.c:21: /usr/include/machine/endian.h:105: warning: `ntohl' redefined ../../include/winsock.h:87: warning: this is the location of the previous definition /usr/include/machine/endian.h:106: warning: `ntohs' redefined ../../include/winsock.h:88: warning: this is the location of the previous definition /usr/include/machine/endian.h:107: warning: `htonl' redefined ../../include/winsock.h:85: warning: this is the location of the previous definition /usr/include/machine/endian.h:108: warning: `htons' redefined ../../include/winsock.h:86: warning: this is the location of the previous definition
(Do you see any way to avoid these warnings? They're quite numerious and hiding real portability problems I'm currently tracking down.)
Thanks for fixing this so quickly! Gerald
On Wed, 2 Apr 2003, Gerald Pfeifer wrote:
Yes, thanks a lot! Now it builds, though I'm getting warnings:
In file included from /usr/include/sys/types.h:126, from ../../include/winsock.h:89, from ../../include/winsock2.h:47, from ../../include/windows.h:62, from icinfo.c:21: /usr/include/machine/endian.h:105: warning: `ntohl' redefined ../../include/winsock.h:87: warning: this is the location of the previous definition /usr/include/machine/endian.h:106: warning: `ntohs' redefined ../../include/winsock.h:88: warning: this is the location of the previous definition /usr/include/machine/endian.h:107: warning: `htonl' redefined ../../include/winsock.h:85: warning: this is the location of the previous definition /usr/include/machine/endian.h:108: warning: `htons' redefined ../../include/winsock.h:86: warning: this is the location of the previous definition
I wonder why this doesn't seem to happen on GNU/Linux; any ideas? In fact, why do we need to override the operating systeme here? Shouldn't these functions be the same?
This really kills my build logs; dozens of message like the above...
Gerald
Gerald Pfeifer pfeifer@dbai.tuwien.ac.at writes:
I wonder why this doesn't seem to happen on GNU/Linux; any ideas? In fact, why do we need to override the operating systeme here? Shouldn't these functions be the same?
Unfortunately Microsoft in their infinite wisdom decided to make the socket functions WINAPI, so we really need the right prototypes. The problem is that they are defined as macros on FreeBSD; we wouldn't need to override them in that case since there is no WINAPI issue with macros, but of course we don't know before including the file whether they will be macros or functions... I think the only solution would be to make our implementation use macros too, even though it's not the way Microsoft does it.
On Wed, 9 Apr 2003, Alexandre Julliard wrote:
Unfortunately Microsoft in their infinite wisdom decided to make the socket functions WINAPI, so we really need the right prototypes.
Ugh.
The problem is that they are defined as macros on FreeBSD; we wouldn't need to override them in that case since there is no WINAPI issue with macros, but of course we don't know before including the file whether they will be macros or functions... I think the only solution would be to make our implementation use macros too, even though it's not the way Microsoft does it.
Thanks for the clear and detailed explanation, Alexandre!
I am not very proud of the patch below, but would that be an alternate solution you could accept, giving your explanation above? (I successfully tested it on FreeBSD, and it shouldn't affect any other platform at all.)
Gerald
ChangeLog: On FreeBSD, where htonl, htons, ntohl, and ntohs are macros, we won't have issues with WINAP, so we can use the definitions provided by the system.
Index: include/winsock.h =================================================================== RCS file: /home/wine/wine/include/winsock.h,v retrieving revision 1.56 diff -u -3 -p -r1.56 winsock.h --- include/winsock.h 9 Apr 2003 23:33:50 -0000 1.56 +++ include/winsock.h 10 Apr 2003 14:49:55 -0000 @@ -56,10 +56,16 @@ # define FD_ISSET Include_winsock_h_before_stdlib_h_or_use_the_MSVCRT_library # define fd_set Include_winsock_h_before_stdlib_h_or_use_the_MSVCRT_library # define select Include_winsock_h_before_stdlib_h_or_use_the_MSVCRT_library -# define htonl Include_winsock_h_before_stdlib_h_or_use_the_MSVCRT_library -# define htons Include_winsock_h_before_stdlib_h_or_use_the_MSVCRT_library -# define ntohl Include_winsock_h_before_stdlib_h_or_use_the_MSVCRT_library -# define ntohs Include_winsock_h_before_stdlib_h_or_use_the_MSVCRT_library +/* In FreeBSD we get the following four from /usr/include/sys/types.h, where + * they are macros, so there is no issue with WINAPI and we can safely use the + * macros. + */ +# ifndef __FreeBSD__ +# define htonl Include_winsock_h_before_stdlib_h_or_use_the_MSVCRT_library +# define htons Include_winsock_h_before_stdlib_h_or_use_the_MSVCRT_library +# define ntohl Include_winsock_h_before_stdlib_h_or_use_the_MSVCRT_library +# define ntohs Include_winsock_h_before_stdlib_h_or_use_the_MSVCRT_library +# endif # else /* FD_CLR */ /* stdlib.h has not been included yet so it's not too late. Include it now * making sure that none of the select symbols is affected. Then we can @@ -68,19 +74,23 @@ # define fd_set unix_fd_set # define timeval unix_timeval # define select unix_select -# define htonl unix_htonl -# define htons unix_htons -# define ntohl unix_ntohl -# define ntohs unix_ntohs +# ifndef __FreeBSD__ +# define htonl unix_htonl +# define htons unix_htons +# define ntohl unix_ntohl +# define ntohs unix_ntohs +# endif # include <sys/types.h> # include <stdlib.h> # undef fd_set # undef timeval # undef select -# undef htonl -# undef htons -# undef ntohl -# undef ntohs +# ifndef __FreeBSD__ +# undef htonl +# undef htons +# undef ntohl +# undef ntohs +# endif # undef FD_SETSIZE # undef FD_CLR # undef FD_SET @@ -481,10 +491,12 @@ typedef struct WS(timeval) #define WS_FD_ISSET(fd, set) __WSAFDIsSet((SOCKET)(fd), (WS_fd_set*)(set)) #endif
+#ifndef __FreeBSD__ u_long WINAPI WS(htonl)(u_long); u_short WINAPI WS(htons)(u_short); u_long WINAPI WS(ntohl)(u_long); u_short WINAPI WS(ntohs)(u_short); +#endif
#endif /* WS_DEFINE_SELECT */
Alexandre,
On Thu, 10 Apr 2003, Gerald Pfeifer wrote:
I am not very proud of the patch below, but would that be an alternate solution you could accept, giving your explanation above? [...] ChangeLog: On FreeBSD, where htonl, htons, ntohl, and ntohs are macros, we won't have issues with WINAP, so we can use the definitions provided by the system.
I noticed that you now committed a different patch of your -- As I said, I fully understood that the one I proposed was ugly. :-) -- and I'm happy to report that
revision 1.57 date: 2003/04/17 02:47:17; author: julliard; state: Exp; lines: +33 -17 Yet another attempt at fixing the htonl functions.
completely solved the problem on FreeBSD.
Thanks!
Gerald
-
Alexandre Julliard -
Gerald Pfeifer