shrink mog_packaddr and improve portability

We cannot assume sa_family_t is the first element of "struct
sockaddr_in" or "struct sockaddr_in6".  FreeBSD has a "sa_len"
member as the first element while Linux does not.

So only keep the parts of the "struct sockaddr*" we need and use
inet_ntop instead of getnameinfo.  This also gives us a little more
space to add additional fields to "struct mog_http" in the future
without increasing memory (or CPU cache) use.

10 files changed, 83 insertions, 98 deletions

diff --git a/Makefile.am b/Makefile.am
index ee8a442..a0c0989 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -70,7 +70,6 @@ mog_src += nameinfo.c
 mog_src += nostd/setproctitle.h
 mog_src += notify.c
 mog_src += notify.h
-mog_src += packaddr.c
 mog_src += packaddr.h
 mog_src += path_parser.h
 mog_src += pidfile.c
diff --git a/accept_loop.c b/accept_loop.c
index dd9c929..874e6d7 100644
--- a/accept_loop.c
+++ b/accept_loop.c
@@ -98,7 +98,7 @@ void *mog_accept_loop(void *arg)
                 client_fd = mog_accept_fn(accept_fd, sa, &salen);
 
                 if (client_fd >= 0)
-                        ac->post_accept_fn(client_fd, ac->svc, sa, salen);
+                        ac->post_accept_fn(client_fd, ac->svc, &msa, salen);
                 else
                         accept_error_check(ac);
         }
diff --git a/cmogstored.h b/cmogstored.h
index 7b1f164..580de8d 100644
--- a/cmogstored.h
+++ b/cmogstored.h
@@ -235,7 +235,7 @@ struct mog_queue {
 
 /* accept.c */
 typedef void (*mog_post_accept_fn)(int fd, struct mog_svc *,
-                                struct sockaddr *, socklen_t);
+                                union mog_sockaddr *, socklen_t);
 struct mog_accept {
         struct mog_svc *svc;
         mog_post_accept_fn post_accept_fn;
@@ -414,7 +414,7 @@ void mog_thrpool_set_size(struct mog_thrpool *, size_t size);
 /* mgmt.c */
 void mog_mgmt_writev(struct mog_mgmt *, struct iovec *, int iovcnt);
 void mog_mgmt_post_accept(int fd, struct mog_svc *,
-                                struct sockaddr *, socklen_t);
+                                union mog_sockaddr *, socklen_t);
 enum mog_next mog_mgmt_queue_step(struct mog_fd *) MOG_CHECK;
 void mog_mgmt_quit_step(struct mog_fd *);
 
@@ -469,9 +469,9 @@ enum mog_next mog_http_get_in_progress(struct mog_fd *);
 
 /* http.c */
 void mog_http_post_accept(int fd, struct mog_svc *,
-                                struct sockaddr *, socklen_t);
+                                union mog_sockaddr *, socklen_t);
 void mog_httpget_post_accept(int fd, struct mog_svc *,
-                                struct sockaddr *, socklen_t);
+                                union mog_sockaddr *, socklen_t);
 enum mog_next mog_http_queue_step(struct mog_fd *) MOG_CHECK;
 void mog_http_quit_step(struct mog_fd *);
 char *mog_http_path(struct mog_http *, char *buf);
@@ -582,6 +582,7 @@ pid_t mog_upgrade_spawn(void);
 /* exit.c */
 _Noreturn void cmogstored_exit(void);
 
+verify(sizeof(in_port_t) <= sizeof(uint16_t));
 /*
  * We only deal with ipv4 and ipv6 addresses (and no human-friendly
  * hostnames/service names), so we can use smaller constants than the
@@ -594,4 +595,4 @@ struct mog_ni {
 };
 
 /* nameinfo.c */
-int mog_nameinfo(struct sockaddr *, socklen_t, struct mog_ni *);
+void mog_nameinfo(struct mog_packaddr *, struct mog_ni *);
diff --git a/http.c b/http.c
index 3f20f60..74ca982 100644
--- a/http.c
+++ b/http.c
@@ -279,46 +279,47 @@ void mog_http_quit_step(struct mog_fd *mfd)
 
 /* stringify the address for tracers */
 static MOG_NOINLINE void
-trace_http_accepted(struct mog_fd *mfd, struct sockaddr *sa, socklen_t salen)
+trace_http_accepted(struct mog_fd *mfd)
 {
 #ifdef HAVE_SYSTEMTAP
+        struct mog_packaddr *mpa = &mfd->as.http.mpa;
         struct mog_ni ni;
-        int rc = mog_nameinfo(sa, salen, &ni);
-        const char *host = rc == 0 ? ni.ni_host : gai_strerror(rc);
 
-        TRACE(CMOGSTORED_HTTP_ACCEPTED(mfd->fd, host, ni.ni_serv));
+        mog_nameinfo(mpa, &ni);
+        TRACE(CMOGSTORED_HTTP_ACCEPTED(mfd->fd, ni.ni_host, ni.ni_serv));
 #endif /* !HAVE_SYSTEMTAP */
 }
 
 static void http_post_accept_common(struct mog_fd *mfd, struct mog_svc *svc,
-                        struct sockaddr *sa, socklen_t salen)
+                        union mog_sockaddr *msa, socklen_t salen)
 {
         struct mog_http *http = &mfd->as.http;
 
+        mog_http_init(http, svc);
+        mog_packaddr_init(&http->mpa, msa, salen);
+
         if (TRACE_ENABLED(CMOGSTORED_HTTP_ACCEPTED))
-                trace_http_accepted(mfd, sa, salen);
+                trace_http_accepted(mfd);
 
-        mog_http_init(http, svc);
-        mog_packaddr_init(&http->mpa, sa, salen);
         mog_idleq_add(svc->queue, mfd, MOG_QEV_RD);
 }
 
 /* called immediately after accept(), this initializes the mfd (once) */
 void mog_http_post_accept(int fd, struct mog_svc *svc,
-                        struct sockaddr *sa, socklen_t salen)
+                        union mog_sockaddr *msa, socklen_t salen)
 {
         struct mog_fd *mfd = mog_fd_init(fd, MOG_FD_TYPE_HTTP);
 
-        http_post_accept_common(mfd, svc, sa, salen);
+        http_post_accept_common(mfd, svc, msa, salen);
 }
 
 /* called immediately after accept(), this initializes the mfd (once) */
 void mog_httpget_post_accept(int fd, struct mog_svc *svc,
-                        struct sockaddr *sa, socklen_t salen)
+                        union mog_sockaddr *msa, socklen_t salen)
 {
         struct mog_fd *mfd = mog_fd_init(fd, MOG_FD_TYPE_HTTPGET);
 
-        http_post_accept_common(mfd, svc, sa, salen);
+        http_post_accept_common(mfd, svc, msa, salen);
 }
 
 /*
diff --git a/http_put.c b/http_put.c
index 9974c93..3ff7166 100644
--- a/http_put.c
+++ b/http_put.c
@@ -444,18 +444,12 @@ MOG_NOINLINE static void read_err_dbg(struct mog_fd *mfd, ssize_t r)
 {
         int save_errno = errno;
         struct mog_ni ni;
-        struct sockaddr *sa;
-        const char *addr;
         const char *path = "(unknown)";
         long long bytes = -1;
         const char *errfmt;
-        socklen_t len;
         unsigned last = last_data_recv(mfd->fd);
-        int rc;
 
-        mog_packaddr_read(&mfd->as.http.mpa, &sa, &len);
-        rc = mog_nameinfo(sa, len, &ni);
-        addr = rc == 0 ? ni.ni_host : gai_strerror(rc);
+        mog_nameinfo(&mfd->as.http.mpa, &ni);
 
         if (mfd->as.http.forward) {
                 path = mfd->as.http.forward->as.file.path;
@@ -466,11 +460,11 @@ MOG_NOINLINE static void read_err_dbg(struct mog_fd *mfd, ssize_t r)
         errfmt = (r == 0) ? PFX"premature EOF" : PFX"%m";
 #undef PFX
         errno = save_errno;
-        syslog(LOG_ERR, errfmt, path, addr, ni.ni_serv, bytes);
+        syslog(LOG_ERR, errfmt, path, ni.ni_host, ni.ni_serv, bytes);
 
         if (last != (unsigned)-1)
                 syslog(LOG_ERR, "last_data_recv=%ums from %s%s for PUT %s",
-                       last, addr, ni.ni_serv, path);
+                       last, ni.ni_host, 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 75d3050..d2f67f0 100644
--- a/inherit.c
+++ b/inherit.c
@@ -37,16 +37,20 @@ static void register_listen_fd(int fd)
         struct listener tmp;
         struct listener *ins;
         struct mog_ni ni;
-        int rc;
+        struct mog_packaddr mpa;
         struct sockaddr *sa = mog_sockaddr_sa(&tmp.msa);
 
         tmp.len = (socklen_t)sizeof(tmp.msa);
         if (getsockname(fd, sa, &tmp.len) != 0)
                 die_errno("getsockname(fd=%d) failed", fd);
 
-        rc = mog_nameinfo(sa, tmp.len, &ni);
-        if (rc != 0)
-                die("getnameinfo failed: %s (fd=%d)", gai_strerror(rc), fd);
+        if (sa->sa_family != AF_INET && sa->sa_family != AF_INET6)
+                die("invalid address family=%d (not AF_INET/AF_INET6)",
+                    (int)sa->sa_family);
+
+        mog_packaddr_init(&mpa, &tmp.msa, tmp.len);
+
+        mog_nameinfo(&mpa, &ni);
 
         syslog(LOG_INFO, "inherited %s%s on fd=%d", ni.ni_host, ni.ni_serv, fd);
 
diff --git a/mgmt.c b/mgmt.c
index 503398d..9909d27 100644
--- a/mgmt.c
+++ b/mgmt.c
@@ -275,26 +275,27 @@ void mog_mgmt_quit_step(struct mog_fd *mfd)
 
 /* stringify the address for tracers */
 static MOG_NOINLINE void
-trace_mgmt_accepted(struct mog_fd *mfd, struct sockaddr *sa, socklen_t salen)
+trace_mgmt_accepted(
+        struct mog_fd *mfd, union mog_sockaddr *msa, socklen_t salen)
 {
 #ifdef HAVE_SYSTEMTAP
+        struct mog_packaddr mpa;
         struct mog_ni ni;
-        int rc = mog_nameinfo(sa, salen, &ni);
-        const char *host = rc == 0 ? ni.ni_host : gai_strerror(rc);
 
-        TRACE(CMOGSTORED_MGMT_ACCEPTED(mfd->fd, host, ni.ni_serv));
+        mog_nameinfo(&mpa, &ni);
+        TRACE(CMOGSTORED_MGMT_ACCEPTED(mfd->fd, ni.ni_host, ni.ni_serv));
 #endif /* !HAVE_SYSTEMTAP */
 }
 
 /* called immediately after accept(), this initializes the mfd (once) */
 void mog_mgmt_post_accept(int fd, struct mog_svc *svc,
-                        struct sockaddr *sa, socklen_t salen)
+                        union mog_sockaddr *msa, socklen_t salen)
 {
         struct mog_fd *mfd = mog_fd_init(fd, MOG_FD_TYPE_MGMT);
         struct mog_mgmt *mgmt = &mfd->as.mgmt;
 
         if (TRACE_ENABLED(CMOGSTORED_MGMT_ACCEPTED))
-                trace_mgmt_accepted(mfd, sa, salen);
+                trace_mgmt_accepted(mfd, msa, salen);
 
         mog_mgmt_init(mgmt, svc);
         mog_idleq_add(svc->queue, mfd, MOG_QEV_RD);
diff --git a/nameinfo.c b/nameinfo.c
index 5d9122f..f5af802 100644
--- a/nameinfo.c
+++ b/nameinfo.c
@@ -3,45 +3,46 @@
  * License: GPLv3 or later (see COPYING for details)
  */
 #include "cmogstored.h"
+#include <arpa/inet.h>
 
 /*
- * small wrapper around getnameinfo(3), this only handles numeric types
+ * small replacement for 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(struct sockaddr *sa, socklen_t len, struct mog_ni *ni)
+void mog_nameinfo(struct mog_packaddr *mpa, 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 ':' */
-
-        int flags = NI_NUMERICHOST | NI_NUMERICSERV;
         int rc;
+        const void *src;
+        const char *ret;
 
-        if (sa->sa_family == AF_INET6) {
+        if (mpa->sa_family == AF_INET6) {
                 hostptr[0] = '['; /* leading '[' */
+                src = mpa->as.in6_ptr;
                 hostptr++;
+        } else {
+                assert(mpa->sa_family == AF_INET && "bad family");
+                src = &mpa->as.in_addr;
         }
 
-        rc = getnameinfo(sa, len, hostptr, hostlen, servptr, servlen, flags);
+        ret = inet_ntop(mpa->sa_family, src, hostptr, (socklen_t)hostlen);
 
         /* terminate serv string on error */
-        if (rc != 0) {
-                ni->ni_serv[0] = 0;
-        } else {
-                ni->ni_serv[0] = ':';
-
-                /* add trailing ']' */
-                if (sa->sa_family == AF_INET6) {
-                        hostlen = strlen(hostptr);
-                        hostptr[hostlen] = ']';
-                        hostptr[hostlen + 1] = 0;
-                }
+        assert(ret == hostptr && "inet_ntop");
+        ni->ni_serv[0] = ':';
+
+        /* add trailing ']' */
+        if (mpa->sa_family == AF_INET6) {
+                hostlen = strlen(hostptr);
+                hostptr[hostlen] = ']';
+                hostptr[hostlen + 1] = 0;
         }
 
-        return rc;
+        /* port number */
+        rc = snprintf(servptr, servlen, "%u", (unsigned)ntohs(mpa->port));
+        assert(rc > 0 && rc < (int)servlen && "we suck at snprintf");
 }
diff --git a/packaddr.c b/packaddr.c
deleted file mode 100644
index 75450d2..0000000
--- a/packaddr.c
+++ /dev/null
@@ -1,21 +0,0 @@
-/*
- * Copyright (C) 2012-2013, Eric Wong <normalperson@yhbt.net>
- * License: GPLv3 or later (see COPYING for details)
- */
-#include "cmogstored.h"
-
-void mog_packaddr_read(struct mog_packaddr *mpa,
-                        struct sockaddr **sa, socklen_t *salen)
-{
-        switch (mpa->as.sa_family) {
-        case AF_INET:
-                *salen = (socklen_t)sizeof(struct sockaddr_in);
-                *sa = (struct sockaddr *)&mpa->as.in4;
-                return;
-        case AF_INET6:
-                *salen = (socklen_t)sizeof(struct sockaddr_in6);
-                *sa = mog_sockaddr_sa(mpa->as.msa.ptr);
-                return;
-        }
-        assert(0 && "unrecognized sa_family" && mpa->as.sa_family);
-}
diff --git a/packaddr.h b/packaddr.h
index 4c9d880..fdfb9cb 100644
--- a/packaddr.h
+++ b/packaddr.h
@@ -3,9 +3,11 @@
  * License: GPLv3 or later (see COPYING for details)
  */
 
-/* avoid sockaddr_storage since that bigger than we need */
+/*
+ * avoid sockaddr_storage since that bigger than we need
+ * This is meant to be cast to "struct sockaddr" via mog_sockaddr_sa
+ */
 union mog_sockaddr {
-        sa_family_t sa_family;
         struct sockaddr_in in;
         struct sockaddr_in6 in6;
         uint8_t bytes[sizeof(struct sockaddr_in6)];
@@ -17,33 +19,36 @@ static inline struct sockaddr *mog_sockaddr_sa(union mog_sockaddr *msa)
         return (struct sockaddr *)msa;
 }
 
+/* this is the relevant part we may store in "struct mog_fd" */
 struct mog_packaddr {
+        sa_family_t sa_family;
+        in_port_t port;
         union {
-                sa_family_t sa_family;
-                struct sockaddr_in in4;
-                struct {
-                        sa_family_t sa_family; /* padding */
-                        union mog_sockaddr *ptr;
-                } msa;
+                struct in6_addr *in6_ptr;
+                struct in_addr in_addr;
         } as;
 } __attribute__((packed));
 
 static inline void mog_packaddr_free(struct mog_packaddr *mpa)
 {
-        if (mpa->as.sa_family != AF_INET)
-                free(mpa->as.msa.ptr);
+        if (mpa->sa_family != AF_INET)
+                free(mpa->as.in6_ptr);
 }
 
 static inline void mog_packaddr_init(struct mog_packaddr *mpa,
-                                struct sockaddr *sa, socklen_t salen)
+                                union mog_sockaddr *msa, socklen_t salen)
 {
-        if (sa->sa_family == AF_INET) {
-                memcpy(&mpa->as.in4, sa, (size_t)salen);
+        mpa->sa_family = msa->in.sin_family;
+
+        if (mpa->sa_family == AF_INET) {
+                mpa->port = msa->in.sin_port;
+                memcpy(&mpa->as.in_addr, &msa->in.sin_addr,
+                        sizeof(struct in_addr));
         } else {
-                mpa->as.sa_family = sa->sa_family;
-                mpa->as.msa.ptr = xmalloc(sizeof(union mog_sockaddr));
-                memcpy(mpa->as.msa.ptr, sa, (size_t)salen);
+                assert(mpa->sa_family == AF_INET6 && "invalid sa_family");
+                mpa->port = msa->in6.sin6_port;
+                mpa->as.in6_ptr = xmalloc(sizeof(struct in6_addr));
+                memcpy(mpa->as.in6_ptr, &msa->in6.sin6_addr,
+                        sizeof(struct in6_addr));
         }
 }
-
-void mog_packaddr_read(struct mog_packaddr *, struct sockaddr **, socklen_t *);