Add EXTRACT_ macros/functions for IPv4 addresses, get rid of structure wrappers.

Add EXTRACT_IPV4_TO_HOST_ORDER() and EXTRACT_IPV4_TO_NETWORK_ORDER();
the former extracts a possibly-unaligned IPv4 address, in network byte
order, returning a uint32_t in host byte order, and the latter extracts
a possibly-unaligned IPv4 address, in network byte order, returning a
uint32_t in *network* byte order.  Some APIs take an address in network
byte order, and some operations are more easily done in host byte order,
so both are useful.

Remove the structure wrappers around nd_ipv4 and nd_ipv6; that makes it
easier to pass variables of those types to functions/macros that take a
byte pointer as an argument (because they might be used either with
pointers to structure members or raw buffer pointers), and the structure
probably wouldn't do much to prevent people from using EXTRACT_BE_U_4()
when they really want to extract the value in *network* byte order;
using the above EXTRACT_IPV4_ calls should do more to encourage that.

cpack.c

@@ -35,9 +35,11 @@
 #include <string.h>
 #include <netdissect-stdinc.h>
 
-#include "cpack.h"
+#include "netdissect.h"
 #include "extract.h"
 
+#include "cpack.h"
+
 const uint8_t *
 cpack_next_boundary(const uint8_t *buf, const uint8_t *p, size_t alignment)
 {

extract.h

@@ -19,6 +19,8 @@
  * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  */
 
+#include <string.h>
+
 /*
  * For 8-bit values; needed to fetch a one-byte value.  Byte order
  * isn't relevant, and alignment isn't an issue.
@@ -121,6 +123,16 @@ EXTRACT_BE_S_8(const void *p)
 		((uint64_t)ntohl(*((const uint32_t *)(p) + 1))) << 0));
 
 }
+
+/*
+ * Extract an IPv4 address, which is in network byte order, and not
+ * necessarily aligned, and provide the result in host byte order.
+ */
+static inline uint32_t UNALIGNED_OK
+EXTRACT_IPV4_TO_HOST_ORDER(const void *p)
+{
+	return ((uint32_t)ntohl(*(const uint32_t *)(p)));
+}
 #elif defined(__GNUC__) && defined(HAVE___ATTRIBUTE__) && \
     (defined(__alpha) || defined(__alpha__) || \
      defined(__mips) || defined(__mips__))
@@ -228,6 +240,16 @@ EXTRACT_BE_S_8(const void *p)
 	return ((int64_t)(((uint64_t)ntohl(((const unaligned_uint32_t *)(p) + 0)->val)) << 32 |
 		((uint64_t)ntohl(((const unaligned_uint32_t *)(p) + 1)->val)) << 0));
 }
+
+/*
+ * Extract an IPv4 address, which is in network byte order, and not
+ * necessarily aligned, and provide the result in host byte order.
+ */
+UNALIGNED_OK static inline uint32_t
+EXTRACT_IPV4_TO_HOST_ORDER(const void *p)
+{
+	return ((uint32_t)ntohl(((const unaligned_uint32_t *)(p))->val));
+}
 #else
 /*
  * This architecture doesn't natively support unaligned loads, and either
@@ -271,8 +293,37 @@ EXTRACT_BE_S_8(const void *p)
 	           ((uint64_t)(*((const uint8_t *)(p) + 5)) << 16) | \
 	           ((uint64_t)(*((const uint8_t *)(p) + 6)) << 8) | \
 	           ((uint64_t)(*((const uint8_t *)(p) + 7)) << 0)))
+
+/*
+ * Extract an IPv4 address, which is in network byte order, and not
+ * necessarily aligned, and provide the result in host byte order.
+ */
+#define EXTRACT_IPV4_TO_HOST_ORDER(p) \
+	((uint32_t)(((uint32_t)(*((const uint8_t *)(p) + 0)) << 24) | \
+	            ((uint32_t)(*((const uint8_t *)(p) + 1)) << 16) | \
+	            ((uint32_t)(*((const uint8_t *)(p) + 2)) << 8) | \
+	            ((uint32_t)(*((const uint8_t *)(p) + 3)) << 0)))
 #endif /* unaligned access checks */
 
+/*
+ * Extract an IPv4 address, which is in network byte order, and which
+ * is not necessarily aligned on a 4-byte boundary, and provide the
+ * result in network byte order.
+ *
+ * This works the same way regardless of the host's byte order.
+ */
+static inline uint32_t
+EXTRACT_IPV4_TO_NETWORK_ORDER(const void *p)
+{
+	uint32_t addr;
+
+	UNALIGNED_MEMCPY(&addr, p, sizeof(uint32_t));
+	return addr;
+}
+
+/*
+ * Non-power-of-2 sizes.
+ */
 #define EXTRACT_BE_U_3(p) \
 	((uint32_t)(((uint32_t)(*((const uint8_t *)(p) + 0)) << 16) | \
 	            ((uint32_t)(*((const uint8_t *)(p) + 1)) << 8) | \

netdissect.h

@@ -67,31 +67,33 @@ typedef unsigned char nd_int32_t[4];
  *
  * It's defined as an array of octets, so that it's not guaranteed to
  * be aligned on its "natural" boundary (in some packet formats, it
- * *isn't* so aligned), and it's defined as a structure in the hopes
- * that this makes it harder to naively use EXTRACT_BE_U_4() to extract
- * the value - in many cases you just want to use UNALIGNED_MEMCPY() to
- * copy its value, so that it remains in network byte order.
+ * *isn't* so aligned).  We have separate EXTRACT_ calls for them;
+ * sometimes you want the host-byte-order value, other times you want
+ * the network-byte-order value.
  *
- * (Among other things, we don't want somebody thinking "IPv4 addresses,
- * they're in network byte order, so we want EXTRACT_BE_U_4(), right?"
- * and then handing the result to system APIs that expect network-order
- * IPv4 addresses, such as inet_ntop(), on their little-endian PCs, getting
- * the wrong behavior, and concluding "oh, it must be in *little*-endian
- * order" and "fixing" it to use EXTRACT_LE_U_4().  Yes, people do this;
- * that's why Wireshark has tvb_get_ipv4(), to extract an IPv4 address from
- * a packet data buffer; it was introduced in reaction to somebody who
- * *had* done that.)
+ * Don't use EXTRACT_BE_U_4() on them, use EXTRACT_IPV4_TO_HOST_ORDER()
+ * if you want them in host byte order and EXTRACT_IPV4_TO_NETWORK_ORDER()
+ * if you want them in network byte order (which you want with system APIs
+ * that expect network-order IPv4 addresses, such as inet_ntop()).
+ *
+ * If, on your little-endian machine (e.g., an "IBM-compatible PC", no matter
+ * what the OS, or an Intel Mac, no matter what the OS), you get the wrong
+ * answer, and you've used EXTRACT_BE_U_4(), do *N*O*T* "fix" this by using
+ * EXTRACT_LE_U_4(), fix it by using EXTRACT_IPV4_TO_NETWORK_ORDER(),
+ * otherwise you're breaking the result on big-endian machines (e.g.,
+ * most PowerPC/Power ISA machines, System/390 and z/Architecture, SPARC,
+ * etc.).
+ *
+ * Yes, people do this; that's why Wireshark has tvb_get_ipv4(), to extract
+ * an IPv4 address from a packet data buffer; it was introduced in reaction
+ * to somebody who *had* done that.
  */
-typedef struct {
-	unsigned char bytes[4];
-} nd_ipv4;
+typedef unsigned char nd_ipv4[4];
 
 /*
  * Use this for IPv6 addresses and netmasks.
  */
-typedef struct {
-	unsigned char bytes[16];
-} nd_ipv6;
+typedef unsigned char nd_ipv6[16];
 
 /*
  * Use this for MAC addresses.

print-decnet.c

@@ -38,8 +38,8 @@ struct rtentry;
 #include <stdlib.h>
 #include <string.h>
 
-#include "extract.h"
 #include "netdissect.h"
+#include "extract.h"
 #include "addrtoname.h"
 
 static const char tstr[] = "[|decnet]";

print-ip.c

@@ -98,7 +98,6 @@ ip_finddst(netdissect_options *ndo,
 	int length;
 	int len;
 	const u_char *cp;
-	uint32_t retval;
 
 	cp = (const u_char *)(ip + 1);
 	length = (IP_HL(ip) << 2) - sizeof(struct ip);
@@ -125,13 +124,11 @@ ip_finddst(netdissect_options *ndo,
 		case IPOPT_LSRR:
 			if (len < 7)
 				break;
-			UNALIGNED_MEMCPY(&retval, cp + len - 4, 4);
-			return retval;
+			return (EXTRACT_IPV4_TO_NETWORK_ORDER(cp + len - 4));
 		}
 	}
 trunc:
-	UNALIGNED_MEMCPY(&retval, &ip->ip_dst, sizeof(uint32_t));
-	return retval;
+	return (EXTRACT_IPV4_TO_NETWORK_ORDER(ip->ip_dst));
 }
 
 /*
@@ -155,9 +152,9 @@ nextproto4_cksum(netdissect_options *ndo,
 	ph.len = htons((uint16_t)len);
 	ph.mbz = 0;
 	ph.proto = next_proto;
-	UNALIGNED_MEMCPY(&ph.src, &ip->ip_src, sizeof(uint32_t));
+	ph.src = EXTRACT_IPV4_TO_NETWORK_ORDER(ip->ip_src);
 	if (IP_HL(ip) == 5)
-		UNALIGNED_MEMCPY(&ph.dst, &ip->ip_dst, sizeof(uint32_t));
+		ph.dst = EXTRACT_IPV4_TO_NETWORK_ORDER(ip->ip_dst);
 	else
 		ph.dst = ip_finddst(ndo, ip);