wrap getnameinfo for consistency in error logging

This will allow us to more easily handle error reporting for
IPv6 addresses and allow for consistent formatting of
stringified IP addresses.

5 files changed, 62 insertions, 42 deletions

diff --git a/Makefile.am b/Makefile.am
index 4c9f19a..dc556c5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -66,6 +66,7 @@ mog_src += mkpath_for.c
 mog_src += mnt.c
 mog_src += mnt.h
 mog_src += mnt_usable.c
+mog_src += nameinfo.c
 mog_src += nostd/setproctitle.h
 mog_src += notify.c
 mog_src += notify.h
diff --git a/cmogstored.h b/cmogstored.h
index 25bbb62..170b840 100644
--- a/cmogstored.h
+++ b/cmogstored.h
@@ -551,8 +551,10 @@ _Noreturn void cmogstored_exit(void);
  * standard NI_MAXHOST/NI_MAXSERV values (1025 and 32 respectively).
  * This reduces our per-thread stack usage and keeps caches hotter.
  */
-#define MOG_NI_MAXHOST (INET6_ADDRSTRLEN)
-#define MOG_NI_MAXSERV (sizeof(":65536"))
+struct mog_ni {
+        char ni_host[INET6_ADDRSTRLEN + sizeof("[]") - 1];
+        char ni_serv[sizeof(":65536")];
+};
 
 /* avoid sockaddr_storage since that bigger than we need */
 union mog_sockaddr {
@@ -561,6 +563,9 @@ union mog_sockaddr {
         uint8_t bytes[sizeof(struct sockaddr_in6)];
 };
 
+/* nameinfo.c */
+int mog_nameinfo(union mog_sockaddr *, socklen_t, struct mog_ni *);
+
 static inline struct sockaddr *mog_sockaddr_sa(union mog_sockaddr *msa)
 {
         return (struct sockaddr *)msa;
diff --git a/http_put.c b/http_put.c
index f53a579..a27d389 100644
--- a/http_put.c
+++ b/http_put.c
@@ -440,42 +440,25 @@ static unsigned last_data_recv(int fd)
 
 MOG_NOINLINE static void read_err_dbg(struct mog_fd *mfd, ssize_t r)
 {
+        static const char no_ip[] = "(unconnected)";
+        int save_errno = errno;
         union mog_sockaddr any;
-        char addrbuf[MOG_NI_MAXHOST];
-        char portbuf[MOG_NI_MAXSERV];
+        struct mog_ni ni;
         const char *addr;
-        static const char no_ip[] = "unconnected";
         const char *path = "(unknown)";
         long long bytes = -1;
         const char *errfmt;
-        unsigned last;
+        socklen_t len = (socklen_t)sizeof(any);
+        unsigned last = last_data_recv(mfd->fd);
+        int rc = getpeername(mfd->fd, mog_sockaddr_sa(&any), &len);
 
-        PRESERVE_ERRNO(last = last_data_recv(mfd->fd));
-
-        portbuf[0] = 0;
-        PRESERVE_ERRNO(do {
-                socklen_t len = (socklen_t)sizeof(any);
-                socklen_t addrlen = (socklen_t)sizeof(addrbuf);
-                socklen_t portlen = (socklen_t)(sizeof(portbuf));
-                int rc = getpeername(mfd->fd, mog_sockaddr_sa(&any), &len);
-
-                if (rc == 0) {
-                        rc = getnameinfo(mog_sockaddr_sa(&any), len,
-                                         addrbuf, addrlen,
-                                         portbuf + 1, portlen - 1,
-                                         NI_NUMERICHOST|NI_NUMERICSERV);
-                        if (rc == 0) {
-                                addr = addrbuf;
-                                portbuf[0] = ':';
-                        } else {
-                                addr = gai_strerror(rc);
-                        }
-                } else {
-                        syslog(LOG_ERR, "getpeername() failed for fd=%d: %m",
-                               mfd->fd);
-                        addr = no_ip;
-                }
-        } while (0));
+        if (rc == 0) {
+                rc = mog_nameinfo(&any, len, &ni);
+                addr = rc == 0 ? ni.ni_host : gai_strerror(rc);
+        } else {
+                syslog(LOG_ERR, "getpeername() failed for fd=%d: %m", mfd->fd);
+                addr = no_ip;
+        }
 
         if (mfd->as.http.forward) {
                 path = mfd->as.http.forward->as.file.path;
@@ -485,11 +468,12 @@ MOG_NOINLINE static void read_err_dbg(struct mog_fd *mfd, ssize_t r)
 #define PFX "PUT %s failed from %s%s after %lld bytes: "
         errfmt = (r == 0) ? PFX"premature EOF" : PFX"%m";
 #undef PFX
-        syslog(LOG_ERR, errfmt, path, addr, portbuf, bytes);
+        errno = save_errno;
+        syslog(LOG_ERR, errfmt, path, addr, ni.ni_serv, bytes);
 
         if (last != (unsigned)-1)
                 syslog(LOG_ERR, "last_data_recv=%ums from %s%s for PUT %s",
-                       last, addr, portbuf, path);
+                       last, addr, ni.ni_serv, path);
 }
 
 static enum mog_next identity_put_in_progress(struct mog_fd *mfd)
diff --git a/inherit.c b/inherit.c
index 3ec280c..7110067 100644
--- a/inherit.c
+++ b/inherit.c
@@ -36,21 +36,18 @@ static void register_listen_fd(int fd)
 {
         struct listener tmp;
         struct listener *ins;
-        int flags = NI_NUMERICHOST | NI_NUMERICSERV;
-        char hbuf[MOG_NI_MAXHOST];
-        char sbuf[MOG_NI_MAXSERV];
+        struct mog_ni ni;
         int rc;
 
         tmp.len = (socklen_t)sizeof(tmp.as);
         if (getsockname(fd, mog_sockaddr_sa(&tmp.as), &tmp.len) != 0)
                 die_errno("getsockname(fd=%d) failed", fd);
 
-        rc = getnameinfo(mog_sockaddr_sa(&tmp.as), tmp.len,
-                         hbuf, sizeof(hbuf), sbuf, sizeof(sbuf), flags);
+        rc = mog_nameinfo(&tmp.as, tmp.len, &ni);
         if (rc != 0)
-                die("getnameinfo failed: %s", gai_strerror(rc));
+                die("getnameinfo failed: %s (fd=%d)", gai_strerror(rc), fd);
 
-        syslog(LOG_INFO, "inherited %s:%s on fd=%d", hbuf, sbuf, fd);
+        syslog(LOG_INFO, "inherited %s%s on fd=%d", ni.ni_host, ni.ni_serv, fd);
 
         ins = xmalloc(sizeof(*ins));
         *ins = tmp;
@@ -58,7 +55,8 @@ static void register_listen_fd(int fd)
 
         switch (hash_insert_if_absent(listeners, ins, NULL)) {
         case 0:
-                die("duplicate listener %s:%s on fd=%d", hbuf, sbuf, fd);
+                die("duplicate listener %s%s on fd=%d",
+                        ni.ni_host, ni.ni_serv, fd);
                 break;
         case 1: /* success */
                 return;
diff --git a/nameinfo.c b/nameinfo.c
new file mode 100644
index 0000000..3a086ae
--- /dev/null
+++ b/nameinfo.c
@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2012-2013, Eric Wong <normalperson@yhbt.net>
+ * License: GPLv3 or later (see COPYING for details)
+ */
+#include "cmogstored.h"
+
+/*
+ * small wrapper around getnameinfo(3), this only handles numeric types
+ * for IPv4 and IPv6 and uses the compact mog_ni structure to reduce
+ * stack usage in error reporting.
+ *
+ * returns the return value of getnameinfo(3)
+ */
+int mog_nameinfo(union mog_sockaddr *any, socklen_t len, struct mog_ni *ni)
+{
+        char *hostptr = ni->ni_host;
+        size_t hostlen = sizeof(ni->ni_host) - (sizeof("[]") - 1);
+
+        char *servptr = ni->ni_serv + 1; /* offset for ':' */
+        size_t servlen = sizeof(ni->ni_serv) - 1; /* offset for ':' */
+
+        struct sockaddr *sa = mog_sockaddr_sa(any);
+        int flags = NI_NUMERICHOST | NI_NUMERICSERV;
+        int rc;
+
+        rc = getnameinfo(sa, len, hostptr, hostlen, servptr, servlen, flags);
+
+        /* terminate serv string on error */
+        ni->ni_serv[0] = rc == 0 ? ':' : 0;
+
+        return rc;
+}