[PATCH v3] util, tcp: Add helper to display socket addresses

[PATCH v3] util, tcp: Add helper to display socket addresses

[David Gibson]

When reporting errors, we sometimes want to show a relevant socket address.
Doing so by extracting the various relevant fields can be pretty awkward,
so introduce a sockaddr_ntop() helper to make it simpler.  For now we just
have one user in tcp.c, but I have further upcoming patches which can make
use of it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tcp.c  | 23 +++++++++--------------
 util.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 util.h | 14 ++++++++++++++
 3 files changed, 79 insertions(+), 14 deletions(-)

Changes since v2:
 * Removed use of iprintf() helper function, use some macros instead.
 * Use sizeof("literal") approach to simplify some things.
 
Changes since v1:
 * More careful and documented reasoning about the size of the strings
   needed for output.
 * Some more complex logic to calling inet_ntop() into a temporary
   buffer then copying it

diff --git a/tcp.c b/tcp.c
index 21d0af06..efbbc1c0 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2758,6 +2758,7 @@ static void tcp_tap_conn_from_sock(struct ctx *c, in_port_t dstport,
 void tcp_listen_handler(struct ctx *c, union epoll_ref ref,
 			const struct timespec *now)
 {
+	char sastr[SOCKADDR_STRLEN];
 	union sockaddr_inany sa;
 	socklen_t sl = sizeof(sa);
 	union flow *flow;
@@ -2776,25 +2777,15 @@ void tcp_listen_handler(struct ctx *c, union epoll_ref ref,
 
 		if (IN4_IS_ADDR_UNSPECIFIED(addr) ||
 		    IN4_IS_ADDR_BROADCAST(addr) ||
-		    IN4_IS_ADDR_MULTICAST(addr) || port == 0) {
-			char str[INET_ADDRSTRLEN];
-
-			err("Invalid endpoint from TCP accept(): %s:%hu",
-			    inet_ntop(AF_INET, addr, str, sizeof(str)), port);
-			goto cancel;
-		}
+		    IN4_IS_ADDR_MULTICAST(addr) || port == 0)
+			goto bad_endpoint;
 	} else if (sa.sa_family == AF_INET6) {
 		const struct in6_addr *addr = &sa.sa6.sin6_addr;
 		in_port_t port = sa.sa6.sin6_port;
 
 		if (IN6_IS_ADDR_UNSPECIFIED(addr) ||
-		    IN6_IS_ADDR_MULTICAST(addr) || port == 0) {
-			char str[INET6_ADDRSTRLEN];
-
-			err("Invalid endpoint from TCP accept(): %s:%hu",
-			    inet_ntop(AF_INET6, addr, str, sizeof(str)), port);
-			goto cancel;
-		}
+		    IN6_IS_ADDR_MULTICAST(addr) || port == 0)
+			goto bad_endpoint;
 	}
 
 	if (tcp_splice_conn_from_sock(c, ref.tcp_listen.pif,
@@ -2804,6 +2795,10 @@ void tcp_listen_handler(struct ctx *c, union epoll_ref ref,
 	tcp_tap_conn_from_sock(c, ref.tcp_listen.port, flow, s, &sa, now);
 	return;
 
+bad_endpoint:
+	err("Invalid endpoint from TCP accept(): %s",
+	    sockaddr_ntop(&sa, sastr, sizeof(sastr)));
+
 cancel:
 	flow_alloc_cancel(flow);
 }
diff --git a/util.c b/util.c
index 849fa7f6..d66d366a 100644
--- a/util.c
+++ b/util.c
@@ -553,3 +553,59 @@ int write_remainder(int fd, const struct iovec *iov, int iovcnt, size_t skip)
 
 	return 0;
 }
+
+/** sockaddr_ntop() - Convert a socket address to text format
+ * @sa:		Socket address
+ * @dst:	output buffer, minimum SOCKADDR_STRLEN bytes
+ * @size:	size of buffer at @dst
+ *
+ * Return: On success, a non-null pointer to @dst, NULL on failure
+ */
+const char *sockaddr_ntop(const void *sa, char *dst, socklen_t size)
+{
+	sa_family_t family = ((const struct sockaddr *)sa)->sa_family;
+	socklen_t off = 0;
+
+#define IPRINTF(...)							\
+	do {								\
+		off += snprintf(dst + off, size - off, __VA_ARGS__);	\
+		if (off >= size)					\
+			return NULL;					\
+	} while (0)
+
+#define INTOP(af, addr)							\
+	do {								\
+		if (!inet_ntop((af), (addr), dst + off, size - off))	\
+			return NULL;					\
+		off += strlen(dst + off);				\
+	} while (0)
+
+	switch (family) {
+	case AF_INET: {
+		const struct sockaddr_in *sa4 = sa;
+
+		INTOP(AF_INET, &sa4->sin_addr);
+		IPRINTF(":%hu", ntohs(sa4->sin_port));
+		break;
+	}
+
+	case AF_INET6: {
+		const struct sockaddr_in6 *sa6 = sa;
+
+		IPRINTF("[");
+		INTOP(AF_INET6, &sa6->sin6_addr);
+		IPRINTF("]:%hu", ntohs(sa6->sin6_port));
+		break;
+	}
+
+		/* FIXME: Implement AF_UNIX */
+	default:
+		errno = EAFNOSUPPORT;
+		return NULL;
+	}
+
+#undef IPRINTF
+#undef INTOP
+	
+	return dst;
+}
diff --git a/util.h b/util.h
index 264423bb..c8a357e3 100644
--- a/util.h
+++ b/util.h
@@ -180,6 +180,20 @@ static inline const char *af_name(sa_family_t af)
 	}
 }
 
+#define UINT16_STRLEN		(sizeof("65535"))
+
+/* inet address (- '\0') + port (u16) (- '\0') + ':' + '\0' */
+#define SOCKADDR_INET_STRLEN					\
+	(INET_ADDRSTRLEN-1 + UINT16_STRLEN-1 + sizeof(":"))
+
+/* inet6 address (- '\0') + port (u16) (- '\0') + '[' + ']' + ':' + '\0' */
+#define SOCKADDR_INET6_STRLEN				\
+	(INET6_ADDRSTRLEN-1 + UINT16_STRLEN-1 + sizeof("[]:"))
+
+#define SOCKADDR_STRLEN		MAX(SOCKADDR_INET_STRLEN, SOCKADDR_INET6_STRLEN)
+
+const char *sockaddr_ntop(const void *sa, char *dst, socklen_t size);
+
 /**
  * mod_sub() - Modular arithmetic subtraction
  * @a:		Minued, unsigned value < @m
-- 
@@ -180,6 +180,20 @@ static inline const char *af_name(sa_family_t af)
 	}
 }
 
+#define UINT16_STRLEN		(sizeof("65535"))
+
+/* inet address (- '\0') + port (u16) (- '\0') + ':' + '\0' */
+#define SOCKADDR_INET_STRLEN					\
+	(INET_ADDRSTRLEN-1 + UINT16_STRLEN-1 + sizeof(":"))
+
+/* inet6 address (- '\0') + port (u16) (- '\0') + '[' + ']' + ':' + '\0' */
+#define SOCKADDR_INET6_STRLEN				\
+	(INET6_ADDRSTRLEN-1 + UINT16_STRLEN-1 + sizeof("[]:"))
+
+#define SOCKADDR_STRLEN		MAX(SOCKADDR_INET_STRLEN, SOCKADDR_INET6_STRLEN)
+
+const char *sockaddr_ntop(const void *sa, char *dst, socklen_t size);
+
 /**
  * mod_sub() - Modular arithmetic subtraction
  * @a:		Minued, unsigned value < @m
-- 
2.45.1