The Socket tests were wrong: they were assuming too much.

p4raw-id: //depot/perl@11951

3 files changed, 79 insertions, 30 deletions

diff --git a/ext/Socket/Socket.pm b/ext/Socket/Socket.pm
index 06d8c74fdc..5dbe3e7578 100644
--- a/ext/Socket/Socket.pm
+++ b/ext/Socket/Socket.pm
@@ -61,17 +61,22 @@ In addition, some structure manipulation functions are available:
 =item inet_aton HOSTNAME
 
 Takes a string giving the name of a host, and translates that
-to the 4-byte string (structure). Takes arguments of both
+to an opaque string (struct in_adrr). Takes arguments of both
 the 'rtfm.mit.edu' type and '18.181.0.24'. If the host name
 cannot be resolved, returns undef. For multi-homed hosts (hosts
 with more than one address), the first address found is returned.
 
+For portability do not assume that the result of inet_aton() is 32
+bits wide, that it would contain only the IPv4 address in network
+order.
+
 =item inet_ntoa IP_ADDRESS
 
-Takes a four byte ip address (as returned by inet_aton())
-and translates it into a string of the form 'd.d.d.d'
-where the 'd's are numbers less than 256 (the normal
-readable four dotted number notation for internet addresses).
+Takes a string (an opaque string as returned by inet_aton(), or
+a v-string representing the four octets of the IPv4 address in
+network order) and translates it into a string of the form 'd.d.d.d'
+where the 'd's are numbers less than 256 (the normal readable four
+dotted number notation for internet addresses).
 
 =item INADDR_ANY
 
diff --git a/ext/Socket/Socket.t b/ext/Socket/Socket.t
index 4de061bcfa..94d4fb5599 100755
--- a/ext/Socket/Socket.t
+++ b/ext/Socket/Socket.t
@@ -93,12 +93,12 @@ if((inet_ntoa((unpack_sockaddr_in(pack_sockaddr_in(100,inet_aton("10.250.230.10"
 } else {
     print "not ok 9\n"; 
 }
-print ((inet_aton("10.10.10.10") eq v10.10.10.10) ? "ok 10\n" : "not ok 10\n");
-print ((inet_ntoa(v10.10.10.10) eq '10.10.10.10') ? "ok 11\n" : "not ok 11\n");
+print ((inet_ntoa(inet_aton("10.20.30.40")) eq "10.20.30.40") ? "ok 10\n" : "not ok 10\n");
+print ((inet_ntoa(v10.20.30.40) eq "10.20.30.40") ? "ok 11\n" : "not ok 11\n");
 {
     my ($port,$addr) = unpack_sockaddr_in(pack_sockaddr_in(100,v10.10.10.10));
     print (($port == 100) ? "ok 12\n" : "not ok 12\n");
-    print (($addr eq v10.10.10.10) ? "ok 13\n" : "not ok 13\n");
+    print ((inet_ntoa($addr) eq "10.10.10.10") ? "ok 13\n" : "not ok 13\n");
 }
 				     
 eval { inet_ntoa(v10.20.30.400) };
diff --git a/ext/Socket/Socket.xs b/ext/Socket/Socket.xs
index b048e59d62..73cf320e3d 100644
--- a/ext/Socket/Socket.xs
+++ b/ext/Socket/Socket.xs
@@ -179,6 +179,36 @@ not_here(char *s)
     return -1;
 }
 
+#define PERL_IN_ADDR_S_ADDR_SIZE 4
+
+/*
+* Bad assumptions possible here.
+*
+* 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 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 change the below code
+* to use addr.s_addr instead of addr, you have been warned.)
+*
+* 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.
+*
+* --jhi */
+
 #include "constants.c"
 
 MODULE = Socket		PACKAGE = Socket
@@ -200,10 +230,9 @@ inet_aton(host)
 	}
 
 	ST(0) = sv_newmortal();
-	if (ok) {
+	if (ok)
 		sv_setpvn( ST(0), (char *)&ip_address, sizeof ip_address );
 	}
-	}
 
 void
 inet_ntoa(ip_address_sv)
@@ -216,15 +245,20 @@ 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);
-	if (addrlen != sizeof(addr)) {
-	    croak("Bad arg length for %s, length is %d, should be %d",
-			"Socket::inet_ntoa",
-			addrlen, 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 */
+	ip_address = SvPV(ip_address_sv, addrlen);
+	if (addrlen == sizeof(addr) || addrlen == 4)
+	        addr.s_addr =
+		    (ip_address[0] & 0xFF) << 24 |
+		    (ip_address[1] & 0xFF) << 16 |
+		    (ip_address[2] & 0xFF) <<  8 |
+		    (ip_address[3] & 0xFF);
+	else
+	        croak("Bad arg length for %s, length is %d, should be %d",
+		      "Socket::inet_ntoa",
+		      addrlen, sizeof(addr));
+	/* 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),
@@ -233,10 +267,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
@@ -324,18 +354,32 @@ unpack_sockaddr_un(sun_sv)
 	}
 
 void
-pack_sockaddr_in(port,ip_address)
+pack_sockaddr_in(port, ip_address_sv)
 	unsigned short	port
-	char *	ip_address
+	SV *	ip_address_sv
 	CODE:
 	{
 	struct sockaddr_in sin;
-
+	struct in_addr addr;
+	STRLEN addrlen;
+	char * ip_address;
+	if (DO_UTF8(ip_address_sv) && !sv_utf8_downgrade(ip_address_sv, 1))
+	     croak("Wide character in Socket::pack_sockaddr_in");
+	ip_address = SvPV(ip_address_sv, addrlen);
+	if (addrlen == sizeof(addr) || addrlen == 4)
+	        addr.s_addr =
+		    (ip_address[0] & 0xFF) << 24 |
+		    (ip_address[1] & 0xFF) << 16 |
+		    (ip_address[2] & 0xFF) <<  8 |
+		    (ip_address[3] & 0xFF);
+	else
+	        croak("Bad arg length for %s, length is %d, should be %d",
+		      "Socket::pack_sockaddr_in",
+		      addrlen, sizeof(addr));
 	Zero( &sin, sizeof sin, char );
 	sin.sin_family = AF_INET;
 	sin.sin_port = htons(port);
-	Copy( ip_address, &sin.sin_addr, sizeof sin.sin_addr, char );
-
+	sin.sin_addr.s_addr = htonl(addr.s_addr);
 	ST(0) = sv_2mortal(newSVpvn((char *)&sin, sizeof sin));
 	}
 
@@ -347,7 +391,7 @@ unpack_sockaddr_in(sin_sv)
 	STRLEN sockaddrlen;
 	struct sockaddr_in addr;
 	unsigned short	port;
-	struct in_addr	ip_address;
+	struct in_addr  ip_address;
 	char *	sin = SvPV(sin_sv,sockaddrlen);
 	if (sockaddrlen != sizeof(addr)) {
 	    croak("Bad arg length for %s, length is %d, should be %d",
@@ -366,5 +410,5 @@ unpack_sockaddr_in(sin_sv)
 
 	EXTEND(SP, 2);
 	PUSHs(sv_2mortal(newSViv((IV) port)));
-	PUSHs(sv_2mortal(newSVpvn((char *)&ip_address,sizeof ip_address)));
+	PUSHs(sv_2mortal(newSVpvn((char *)&ip_address, sizeof ip_address)));
 	}