Long-standing problem in Socket module

The Socket module provides an inet_aton() which isn't.  It's only
inet_addr().  The latter can't convert '255.255.255.255' (or
various equivalents).  This despite the fact that Socket.pm
claims that a conversion of that address is meaningful, which it
would be if we were using the real inet_aton().

Also, that address is properly the 'this-LAN' broadcast address,
also known as INADDR_BROADCAST.  That constant is missing from
Socket.xs, even though it provides the bogus INADDR_NONE, which
is merely an artifact of using inet_addr().  The all-ones
broadcast address is one of the few IP addresses which the spec
guarantees will be valid.  It's invalid for TCP, since it's a
broadcast address, but it's valid for ICMP and UDP.

The patch below changes Socket.xs to use a proper inet_aton()
conversion and adds the missing major INADDR constant of
INADDR_BROADCAST.  It includes (as the last item patched) the new
metaconfig unit used to make the changes to Configure,
config_h.SH, and Porting/Glossary.

p5p-msgid: <199612190418.XAA07291@Orb.Nashua.NH.US>

Configure

@@ -304,6 +304,7 @@ d_getppid=''
 d_getprior=''
 d_gnulibc=''
 d_htonl=''
+d_inetaton=''
 d_isascii=''
 d_killpg=''
 d_link=''
@@ -6628,6 +6629,10 @@ set d_strchr; eval $setvar
 val="$vali"
 set d_index; eval $setvar
 
+: check whether inet_aton exists
+set inet_aton d_inetaton
+eval $inlibc
+
 : Look for isascii
 echo " "
 $cat >isascii.c <<'EOCP'
@@ -9926,6 +9931,7 @@ d_gettimeod='$d_gettimeod'
 d_gnulibc='$d_gnulibc'
 d_htonl='$d_htonl'
 d_index='$d_index'
+d_inetaton='$d_inetaton'
 d_isascii='$d_isascii'
 d_killpg='$d_killpg'
 d_link='$d_link'

Porting/Glossary

@@ -312,6 +312,11 @@ d_index (d_strchr.U):
 	This variable conditionally defines HAS_INDEX if index() and
 	rindex() are available for string searching.
 
+d_inetaton (d_inetaton.U):
+	This variable conditionally defines the HAS_INET_ATON symbol, which
+	indicates to the C program that the inet_aton() function is available
+	to parse IP address "dotted-quad" strings.
+
 d_isascii (d_isascii.U):
 	This variable conditionally defines the HAS_ISASCII constant,
 	which indicates to the C program that isascii() is available.

U/d_inetaton.U

@@ -0,0 +1,24 @@
+?RCS: Copyright (c) 1996, Spider Boardman
+?RCS:
+?RCS: You may distribute under the terms of either the GNU General Public
+?RCS: License or the Artistic License, as specified in the README file.
+?RCS:
+?MAKE:d_inetaton: Inlibc
+?MAKE:	-pick add $@ %<
+?S:d_inetaton:
+?S:	This variable conditionally defines the HAS_INET_ATON symbol, which
+?S:	indicates to the C program that the inet_aton() function is available
+?S:	to parse IP address "dotted-quad" strings.
+?S:.
+?C:HAS_INET_ATON:
+?C:	This symbol, if defined, indicates to the C program that the
+?C:	inet_aton() function is available to parse IP address "dotted-quad"
+?C:	strings.
+?C:.
+?H:#$d_inetaton HAS_INET_ATON		/**/
+?H:.
+?LINT:set d_inetaton
+: check whether inet_aton exists
+set inet_aton d_inetaton
+eval $inlibc
+

config_h.SH

@@ -1484,6 +1484,13 @@ sed <<!GROK!THIS! >config.h -e 's!^#undef\(.*/\)\*!/\*#define\1 \*!' -e 's!^#un-
 #$d_getpgrp HAS_GETPGRP		/**/
 #$d_bsdgetpgrp USE_BSD_GETPGRP	/**/
 
+/* HAS_INET_ATON:
+ *	This symbol, if defined, indicates to the C program that the
+ *	inet_aton() function is available to parse IP address "dotted-quad"
+ *	strings.
+ */
+#$d_inetaton HAS_INET_ATON		/**/
+
 /* HAS_SETPGID:
  *	This symbol, if defined, indicates to the C program that 
  *	the setpgid(pid, gpid) function is available to set the

ext/Socket/Socket.pm

@@ -1,7 +1,7 @@
 package Socket;
 
 use vars qw($VERSION @ISA @EXPORT);
-$VERSION = "1.5";
+$VERSION = "1.6";
 
 =head1 NAME
 
@@ -52,7 +52,8 @@ In addition, some structure manipulation functions are available:
 Takes a string giving the name of a host, and translates that
 to the 4-byte string (structure). Takes arguments of both
 the 'rtfm.mit.edu' type and '18.181.0.24'. If the host name
-cannot be resolved, returns undef.
+cannot be resolved, returns undef. For multi-homed hosts (hosts
+with more than one address), the first address found is returned.
 
 =item inet_ntoa IP_ADDRESS
 
@@ -72,6 +73,15 @@ a particular network interface. This wildcard address
 allows you to bind to all of them simultaneously.)
 Normally equivalent to inet_aton('0.0.0.0').
 
+=item INADDR_BROADCAST
+
+Note: does not return a number, but a packed string.
+
+Returns the 4-byte 'this-lan' ip broadcast address.
+This can be useful for some protocols to solicit information
+from all servers on the same LAN cable.
+Normally equivalent to inet_aton('255.255.255.255').
+
 =item INADDR_LOOPBACK
 
 Note - does not return a number.
@@ -83,7 +93,7 @@ to inet_aton('localhost').
 
 Note - does not return a number.
 
-Returns the 4-byte invalid ip address. Normally equivalent
+Returns the 4-byte 'invalid' ip address. Normally equivalent
 to inet_aton('255.255.255.255').
 
 =item sockaddr_in PORT, ADDRESS
@@ -145,7 +155,7 @@ require DynaLoader;
 	inet_aton inet_ntoa pack_sockaddr_in unpack_sockaddr_in
 	pack_sockaddr_un unpack_sockaddr_un
 	sockaddr_in sockaddr_un
-	INADDR_ANY INADDR_LOOPBACK INADDR_NONE
+	INADDR_ANY INADDR_BROADCAST INADDR_LOOPBACK INADDR_NONE
 	AF_802
 	AF_APPLETALK
 	AF_CCITT

ext/Socket/Socket.xs

@@ -30,10 +30,117 @@
 #ifndef INADDR_NONE
 #define INADDR_NONE	0xffffffff
 #endif /* INADDR_NONE */
+#ifndef INADDR_BROADCAST
+#define INADDR_BROADCAST	0xffffffff
+#endif /* INADDR_BROADCAST */
 #ifndef INADDR_LOOPBACK
 #define INADDR_LOOPBACK         0x7F000001
 #endif /* INADDR_LOOPBACK */
 
+#ifndef HAS_INET_ATON
+
+/* 
+ * Check whether "cp" is a valid ascii representation
+ * of an Internet address and convert to a binary address.
+ * Returns 1 if the address is valid, 0 if not.
+ * This replaces inet_addr, the return value from which
+ * cannot distinguish between failure and a local broadcast address.
+ */
+static int
+my_inet_aton(cp, addr)
+register const char *cp;
+struct in_addr *addr;
+{
+	register unsigned long val;
+	register int base;
+	register char c;
+	int nparts;
+	const char *s;
+	unsigned int parts[4];
+	register unsigned int *pp = parts;
+
+	for (;;) {
+		/*
+		 * Collect number up to ``.''.
+		 * Values are specified as for C:
+		 * 0x=hex, 0=octal, other=decimal.
+		 */
+		val = 0; base = 10;
+		if (*cp == '0') {
+			if (*++cp == 'x' || *cp == 'X')
+				base = 16, cp++;
+			else
+				base = 8;
+		}
+		while ((c = *cp) != '\0') {
+			if (isDIGIT(c)) {
+				val = (val * base) + (c - '0');
+				cp++;
+				continue;
+			}
+			if (base == 16 && (s=strchr(hexdigit,c))) {
+				val = (val << 4) + 
+					((s - hexdigit) & 15);
+				cp++;
+				continue;
+			}
+			break;
+		}
+		if (*cp == '.') {
+			/*
+			 * Internet format:
+			 *	a.b.c.d
+			 *	a.b.c	(with c treated as 16-bits)
+			 *	a.b	(with b treated as 24 bits)
+			 */
+			if (pp >= parts + 3 || val > 0xff)
+				return 0;
+			*pp++ = val, cp++;
+		} else
+			break;
+	}
+	/*
+	 * Check for trailing characters.
+	 */
+	if (*cp && !isSPACE(*cp))
+		return 0;
+	/*
+	 * Concoct the address according to
+	 * the number of parts specified.
+	 */
+	nparts = pp - parts + 1;	/* force to an int for switch() */
+	switch (nparts) {
+
+	case 1:				/* a -- 32 bits */
+		break;
+
+	case 2:				/* a.b -- 8.24 bits */
+		if (val > 0xffffff)
+			return 0;
+		val |= parts[0] << 24;
+		break;
+
+	case 3:				/* a.b.c -- 8.8.16 bits */
+		if (val > 0xffff)
+			return 0;
+		val |= (parts[0] << 24) | (parts[1] << 16);
+		break;
+
+	case 4:				/* a.b.c.d -- 8.8.8.8 bits */
+		if (val > 0xff)
+			return 0;
+		val |= (parts[0] << 24) | (parts[1] << 16) | (parts[2] << 8);
+		break;
+	}
+	addr->s_addr = htonl(val);
+	return 1;
+}
+
+#undef inet_aton
+#define inet_aton my_inet_aton
+
+#endif /* ! HAS_INET_ATON */
+
 
 static int
 not_here(s)
@@ -595,15 +702,17 @@ inet_aton(host)
 	{
 	struct in_addr ip_address;
 	struct hostent * phe;
+	int ok;
 
 	if (phe = gethostbyname(host)) {
 		Copy( phe->h_addr, &ip_address, phe->h_length, char );
+		ok = 1;
 	} else {
-        	ip_address.s_addr = inet_addr(host);
+		ok = inet_aton(host, &ip_address);
 	}
 
 	ST(0) = sv_newmortal();
-	if(ip_address.s_addr != INADDR_NONE) {
+	if (ok) {
 		sv_setpvn( ST(0), (char *)&ip_address, sizeof ip_address );
 	}
 	}
@@ -748,3 +857,12 @@ INADDR_NONE()
 	ip_address.s_addr = htonl(INADDR_NONE);
 	ST(0) = sv_2mortal(newSVpv((char *)&ip_address,sizeof ip_address));
 	}
+
+void
+INADDR_BROADCAST()
+	CODE:
+	{
+	struct in_addr	ip_address;
+	ip_address.s_addr = htonl(INADDR_BROADCAST);
+	ST(0) = sv_2mortal(newSVpv((char *)&ip_address,sizeof ip_address));
+	}