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 (executable)
--- 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 (file)
--- 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 (file)
--- 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<perlfunc/substr>.
 
-=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,