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

p4raw-id: //depot/perl@11951

diff --git a/ext/Socket/Socket.pm b/ext/Socket/Socket.pm
index 06d8c74..5dbe3e7 100644 (file)
--- 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 4de061b..94d4fb5 100755 (executable)
--- 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 b048e59..73cf320 100644 (file)
--- 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)));
        }