| msg43600 - (view) |
Author: Tim Rice (tim1470) |
Date: 2003-05-03 23:04 |
| This patch adds code to configure.in to determine the correct data type for socklen_t if socklen_t is not defined by the system. Credit for the socklen_t autoconf magic goes to Albert Chin (china@thewrittenword.com) I've also fixed some sections of Modules/socketmodule.c that should have used socklen_t. |
|
|
| msg43601 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2003-05-04 12:29 |
| Logged In: YES user_id=21627 Why do you #undef _XOPEN_SOURCE? If you "miss data types without this", won't you miss the same data types when compiling socketmodule.c? How does the test work? What kind of failure do you expect when "trying the wrong type? On what platforms has this change be tested? What platforms did the old test fail on? What types do these systems use for socklen_t? Please lose the AC_DEFUN; AFAICT, it is used only once. |
|
|
| msg43602 - (view) |
Author: Tim Rice (tim1470) |
Date: 2003-05-04 17:50 |
| Logged In: YES user_id=618097 Looking at it again, undefing _XOPEN_SOURCE is not a good idea. Yes you'll miss the data types in socketmodule.c too but they will be addressed in another patch to Include/pyport.h which is not included in the configure tests. I haven't taken the time to analize why it works (I didn't write it). It's been working fine on a wide range of platforms for over a year in OpenSSH portable. I wanted to get it in 2.2.2 but it requires autoconf 2.52 or newer. I've personaly tested on Linux, Solaris, UnixWare, OpenServer. UnixWare uses size_t and OpenServer uses int. My new patch gets rid of AC_DEFUN and adds some data type tests. Two of the data type tests (u_char & u_long) are not needed by the socklen_t test but will be needed by a UnixWare 7 patch that I'll post shortly. Expect some fuz in the patch. I'm breaking up a large patch into understanable chunks. |
|
|
| msg43603 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2003-05-04 18:56 |
| Logged In: YES user_id=21627 Why do you need to define u_int? Python does not use this type, so it should be irrelevant whether it is defined or not. |
|
|
| msg43604 - (view) |
Author: Tim Rice (tim1470) |
Date: 2003-05-04 19:05 |
| Logged In: YES user_id=618097 u_int is needed for the socklen_t test by sys/socket.h on SCO OpenServer The u_int test towards the top of configure.in is also needed by patch # 732284 for UnixWare 7 |
|
|
| msg43605 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2003-05-04 19:18 |
| Logged In: YES user_id=21627 Why do you say "is needed for the socklen_t test"? Are you referring to the test in your patch? In this test, u_int is not *used* at all, it is only defined. So not defining it should not cause problems. |
|
|
| msg43606 - (view) |
Author: Tim Rice (tim1470) |
Date: 2003-05-04 19:29 |
| Logged In: YES user_id=618097 Without u_short and u_int the soclklen_t test can NEVER suceed on OpenServer because of a syntax error in sys/socket.h whthout them. Ugly, but you don't seem to like the option of seting define_xopen_source=no on platforms that puilt fine prior to 2.3 |
|
|
| msg43607 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2003-05-04 19:51 |
| Logged In: YES user_id=21627 Now we are getting closer. What is the specific line in sys/socket.h that gives the syntax error? Are there alternative definitions of the (function, I suppose), either in socket.h or in another file? Why is u_int not defined when _XOPEN_SOURCE is? IOW: Are you absolutely, definitely, 100% certain that there is no way to compile sys/socket.h when _XOPEN_SOURCE is defined? That would be a serious bug in the system. Now, if we can establish with certainty that the system is broken beyond repair, it is acceptable to work around the bugs, by not defining _XOPEN_SOURCE. We then need to record the specific release of the system that we found is broken, and we need to document what specifically we found broken about it. For better reproducability, we also want to record the name of the person who determined the system being broken. We do all this in the hope that future system releases will be fixed by the system vendors, and that our work-arounds won't be needed anymore when we stop supporting old releases of the system (at a time when nobody uses those old releases anymore). I am not opposed to setting _define_xopen_source=no in principle (I once was, but am not any longer). I only want a convincing analysis that this is necessary to get essential parts of Python (such as sockets) to work. |
|
|
| msg43608 - (view) |
Author: Tim Rice (tim1470) |
Date: 2003-05-04 20:08 |
| Logged In: YES user_id=618097 There may be a way. Hold off until I test defining _POSIX_SOURCE |
|
|
| msg43609 - (view) |
Author: Tim Rice (tim1470) |
Date: 2003-05-04 21:39 |
| Logged In: YES user_id=618097 That was a waste of time. I mis read sys/types.h on OpenServer u_int and u_short are not defined because they are warped inside #if !defined(_XOPEN_SOURCE) && !defined(_POSIX_SOURCE) #endif /* !defined(_XOPEN_SOURCE) && !defined(_POSIX_SOURCE) */ The error is "/usr/include/sys/socket.h", line 171: error: Syntax error before or at: u_short "/usr/include/sys/socket.h", line 171: error: cannot recover from previous errors .... 167:/* 168: * Structure used by kernel to store most addresses. 169: */ 170:struct sockaddr { 171: u_short sa_family; /* address family */ 172: char sa_data[14]; /* up to 14 bytes of direct address */ 173:}; 174: .... I've modified the patch to be less obtrusive my undefing _XOPEN_SOURCE on OpenServer only. We may end up removing the confdefs2.h parts later if we end up using define_xopen_source=no for OpenServer. As it stands now, with or without this patch, OpenServer doesn't build. This patch is really for SVR4.2 and SVR5 systems. (Ie. UnixWare) |
|
|
| msg43610 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2003-05-05 05:18 |
| Logged In: YES user_id=21627 I have now disabled _XOPEN_SOURCE for this system. Before applying a patch like this one, I'd like to know a) how it works (see my first comment) b) what specific error it fixes on UnixWare. |
|
|
| msg43611 - (view) |
Author: Tim Rice (tim1470) |
Date: 2003-05-10 21:10 |
| Logged In: YES user_id=618097 The errors: UX:acomp: WARNING: "/opt/src/utils/python/python/dist/src/Modules/socketmodule.c", line 1100: argument #3 incompatible with prototype: accept() UX:acomp: WARNING: "/opt/src/utils/python/python/dist/src/Modules/socketmodule.c", line 1319: argument #5 incompatible with prototype: _getsockopt() UX:acomp: WARNING: "/opt/src/utils/python/python/dist/src/Modules/socketmodule.c", line 1337: argument #5 incompatible with prototype: _getsockopt() UX:acomp: WARNING: "/opt/src/utils/python/python/dist/src/Modules/socketmodule.c", line 1571: argument #3 incompatible with prototype: getsockname() UX:acomp: WARNING: "/opt/src/utils/python/python/dist/src/Modules/socketmodule.c", line 1599: argument #3 incompatible with prototype: getpeername() UX:acomp: WARNING: "/opt/src/utils/python/python/dist/src/Modules/socketmodule.c", line 1833: argument #6 incompatible with prototype: recvfrom() UX:acomp: WARNING: "/opt/src/utils/python/python/dist/src/Modules/socketmodule.c", line 2963: statement not reached How it works: First it tests if the system supports socklen_t data type. If not, it loops through compiling a test program and testing the compiler's return code. It tries using int, size_t, unsigned, long, and then "unsigned long". I've personaly used this on UnixWare 2.03, UnixWare 2.1.3, UnixWare 7.x SCO OpenServer 3, SCO OpenServer 5, several flavors of Caldera Linux, RedHat 6.2, Solaris 7, & Solaris 8. Looking at http://marc.theaimsgroup.com/?l=openssh-unix-dev&m=100174342829230&w=2 ..... > tested successfully on: > Solaris 2.5.1, 2.6, 7, 8/SPARC, HP-UX 10.20, 11.00, > Tru64 UNIX 4.0D, 5.0A, 5.1, IRIX 6.2, 6.5, AIX 4.3.2. Rsync also features on the Samba build farm (see below), building and testing on over 20 platforms, so its safe to say its portable... Andrew Bartlett ..... socklen-4.patch is against 10 May 2003 CVS |
|
|
| msg43612 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2003-05-11 06:01 |
| Logged In: YES user_id=21627 So these are all warnings only? I'm not sure I want to risk breaking compilation on other systems to get rid of these warnings this late in the beta cycle. When you say you have used this: Do you mean in combination with OpenSSH, or with Python? On the "how it works" issue: How precisely does it cause the compiler to produce a status code? I.e. what in the test program does it assume to give an error? In particular: Why does it call getpeername? |
|
|
| msg43613 - (view) |
Author: Tim Rice (tim1470) |
Date: 2003-05-11 17:57 |
| Logged In: YES user_id=618097 Yes these are warnings. But then warnings are there so you'll look at the code and correct if necessary. If you're nervous about the autoconf test that's fine. I can always correct pyconfig.h after configure runs. But the Modules/socketmodule.c patch should be applied. The code there is just pain wrong. I've used the autoconf bits in OpenSSH and now in my local Python 2.3 tree. I've been using the socketmodule patch since the 2.2.2 beta days. But 2.2.2 uses autoconf 2.13 so the autoconf bits don't work there. I don't know why Albert chose getpeername for the test. I strongly suspect you could use accept, getsockopt, getsockname, recvfrom, or any socket call that has the length option you're testing for. On UnixWare the test progtram (using int) produces the error identifier redeclared: getpeername causing cc to return with a non 0. |
|
|
| msg43614 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2003-05-12 04:23 |
| Logged In: YES user_id=21627 It is fine to chose getpeername for the test. I don't understand why the test features a *call* to getpeername, if the expected error is an incorrect redeclaration. |
|
|
| msg59187 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2008-01-04 00:16 |
| Martin, should the patch be ported and applied in 2.5 and 2.6? |
|
|
| msg110333 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2010-07-14 22:06 |
| Can this be closed, I can't see anyone going to the trouble of porting a patch prepared for 2.3 over 7 years ago to 2.7 or 3.2? |
|
|
| msg114236 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2010-08-18 16:06 |
| Closing as no reply to . |
|
|