On Tue, 21 May 2024 14:48:03 +1000 David Gibson <david(a)gibson.dropbear.id.au> wrote: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(a)gibson.dropbear.id.au>Applied, I just dropped:--- 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 +...this stray tab. -- Stefano