From: Jarkko Hietaniemi Date: Fri, 7 Sep 2001 03:43:11 +0000 (+0000) Subject: Try to make Socket::inet_ntoa() more robust. X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=dc26df50e51d2d0fb283185fc254d0db5b0934c4;p=p5sagit%2Fp5-mst-13.2.git Try to make Socket::inet_ntoa() more robust. p4raw-id: //depot/perl@11926 --- diff --git a/ext/Socket/Socket.t b/ext/Socket/Socket.t index a90a6a4..4de061b 100755 --- a/ext/Socket/Socket.t +++ b/ext/Socket/Socket.t @@ -33,8 +33,8 @@ if (socket(T,PF_INET,SOCK_STREAM,6)) { print(($read == 0 || $buff eq "hello") ? "ok 3\n" : "not ok 3\n"); } else { - print "# You're allowed to fail tests 2 and 3 if.\n"; - print "# The echo service has been disabled.\n"; + print "# You're allowed to fail tests 2 and 3 if\n"; + print "# the echo service has been disabled.\n"; print "# $!\n"; print "ok 2\n"; print "ok 3\n"; @@ -63,8 +63,8 @@ if( socket(S,PF_INET,SOCK_STREAM,6) ){ print(($read == 0 || $buff eq "olleh") ? "ok 6\n" : "not ok 6\n"); } else { - print "# You're allowed to fail tests 5 and 6 if.\n"; - print "# The echo service has been disabled.\n"; + print "# You're allowed to fail tests 5 and 6 if\n"; + print "# the echo service has been disabled.\n"; print "# $!\n"; print "ok 5\n"; print "ok 6\n"; diff --git a/ext/Socket/Socket.xs b/ext/Socket/Socket.xs index ba8c221..9f60040 100644 --- a/ext/Socket/Socket.xs +++ b/ext/Socket/Socket.xs @@ -216,42 +216,59 @@ inet_ntoa(ip_address_sv) char * ip_address; if (DO_UTF8(ip_address_sv) && !sv_utf8_downgrade(ip_address_sv, 1)) croak("Wide character in Socket::inet_ntoa"); - ip_address = SvPV(ip_address_sv,addrlen); - /* Bad assumptions here. - * - * Bad Assumption 1: struct in_addr has no other fields than - * the s_addr (which is the field we really care about here). + ip_address = SvPV(ip_address_sv, addrlen); + /* + * Bad assumptions possible here. * - * Bad Assumption 2: the s_addr field is the first field - * in struct in_addr (the Copy() assumes that). + * Bad Assumption 1: struct in_addr has no other fields + * than the s_addr (which is the field we care about + * in here, really). However, we can be fed either 4-byte + * addresses (from pack("N", ...), or va.b.c.d, or ...), + * or full struct in_addrs (from e.g. pack_sockaddr_in()), + * which may or may not be 4 bytes in size. * - * Bad Assumption 3: the s_addr field is a simple type - * (such as an int). It can be a bit field, in which - * case using & (address-of) on it or taking sizeof() + * Bad Assumption 2: the s_addr field is a simple type + * (such as an int, u_int32_t). It can be a bit field, + * in which case using & (address-of) on it or taking sizeof() * wouldn't go over too well. (Those are not attempted - * now but in case someone thinks to fix the below uses - * of addr (both in the length check and the Copy()) - * by using addr.s_addr. + * now but in case someone thinks to change the below code + * to use addr.s_addr instead of addr, you have warned.) * - * These bad assumptions currently break UNICOS which has - * struct in_addr struct { u_long st_addr:32; } s_da; - * #define s_addr s_da.st_addr + * Bad Assumption 3: the s_addr is the first field in + * an in_addr, or that its bytes are the first bytes in + * an in_addr. * + * These bad assumptions are wrong in UNICOS which has + * struct in_addr { struct { u_long st_addr:32; } s_da }; + * #define s_addr s_da.st_addr * and u_long is 64 bits. * - * The bold soul attempting to fix this should also - * fix pack_sockaddr_in() to agree. - * - * --jhi - */ - if (addrlen != sizeof(addr)) { - croak("Bad arg length for %s, length is %d, should be %d", - "Socket::inet_ntoa", - addrlen, sizeof(addr)); + * --jhi */ +#define PERL_IN_ADDR_S_ADDR_SIZE 4 +#if INTSIZE == PERL_IN_ADDR_S_ADDR_SIZE + if (addrlen == PERL_IN_ADDR_S_ADDR_SIZE) + addr.s_addr = ntohl(*(int*)ip_address); + else +#endif + { + /* The following could be optimized away if we knew + * during compile time what size is struct in_addr. */ + if (addrlen == sizeof(addr)) + Copy( ip_address, &addr, sizeof addr, char ); + else { + if (PERL_IN_ADDR_S_ADDR_SIZE == sizeof(addr)) + croak("Bad arg length for %s, length is %d, should be %d", + "Socket::inet_ntoa", + addrlen, PERL_IN_ADDR_S_ADDR_SIZE); + else + croak("Bad arg length for %s, length is %d, should be %d or %d", + "Socket::inet_ntoa", + addrlen, PERL_IN_ADDR_S_ADDR_SIZE, sizeof(addr)); + } } - Copy( ip_address, &addr, sizeof addr, char ); -#if defined(__hpux) && defined(__GNUC__) && defined(USE_64_BIT_INT) - /* GCC on HP_UX breaks the call to inet_ntoa --sky */ + /* We could use inet_ntoa() but that is broken + * in HP-UX + GCC + 64bitint (returns "0.0.0.0"), + * so let's use this sprintf() workaround everywhere. */ New(1138, addr_str, 4 * 3 + 3 + 1, char); sprintf(addr_str, "%d.%d.%d.%d", ((addr.s_addr >> 24) & 0xFF), @@ -260,10 +277,6 @@ inet_ntoa(ip_address_sv) ( addr.s_addr & 0xFF)); ST(0) = sv_2mortal(newSVpvn(addr_str, strlen(addr_str))); Safefree(addr_str); -#else - addr_str = inet_ntoa(addr); - ST(0) = sv_2mortal(newSVpvn(addr_str, strlen(addr_str))); -#endif } void diff --git a/pod/perldiag.pod b/pod/perldiag.pod index 5a9eb4e..ff8e0a7 100644 --- a/pod/perldiag.pod +++ b/pod/perldiag.pod @@ -261,7 +261,7 @@ avoid this warning. used as an lvalue, which is pretty strange. Perhaps you forgot to dereference it first. See L. -=item Bad arg length for %s, is %d, should be %d +=item Bad arg length for %s, is %d, should be %s (F) You passed a buffer of the wrong size to one of msgctl(), semctl() or shmctl(). In C parlance, the correct sizes are, respectively,