From: Omar Polo Subject: Re: get rid of got_sockaddr.[ch] To: Stefan Sperling Cc: gameoftrees@openbsd.org Date: Wed, 15 Nov 2023 16:47:45 +0100 On 2023/11/15 14:48:34 +0100, Stefan Sperling wrote: > On Wed, Nov 15, 2023 at 02:31:19PM +0100, Omar Polo wrote: > > These were added due to gotwebd weird struct sockaddr_storage handling. > > Instead, we can save the size reported by getaddrinfo() and not reach > > into the struct sockaddr storage at all (except for extracting the port > > number for diagnostics purposes.) > > > > sockets_conf_new_socket_fcgi() gets an hardcoded ipproto to 0 at the > > moment, and for the moment keeps the hardcoded SOCK_STREAM. cleaning up > > this function will be the next step. > > Seems a->ipproto was always 0 anyway? I don't see it being set anywhere. AFAIK the only valid option for it in practice is PF_UNSPEC (aka 0), so yeah, nothing new. However, I'd still like to clean up sockets_conf_new_socket_fcgi() a bit, saving a few flags from the getaddrinfo() and > OK by me. This is a nice simplification. thanks! here's the follow-up. The main idea behind saving ai_{family,socktype,protocol} from the getaddrinfo() call is also to hopefully merge in the future the two sockets_conf_new_* routines. I haven't tried that yet, be we should be able to keep only one struct sockaddr_storage for the AF_UNIX as well as INET/INT6 usage. this is the only diff in this batch that isn't a net-negative :/ diffstat /home/op/w/gotwebd M gotwebd/gotwebd.h | 3+ 0- M gotwebd/parse.y | 7+ 2- M gotwebd/sockets.c | 8+ 13- 3 files changed, 18 insertions(+), 15 deletions(-) diff /home/op/w/gotwebd commit - cdfd248aa718819d40d0bf972b7efbb2eabd31c9 path + /home/op/w/gotwebd blob - d5fda8da9e59b01cc49feab0b96c7ebec5256b68 file + gotwebd/gotwebd.h --- gotwebd/gotwebd.h +++ gotwebd/gotwebd.h @@ -264,6 +264,9 @@ struct address { TAILQ_ENTRY(address) entry; struct sockaddr_storage ss; socklen_t slen; + int ai_family; + int ai_socktype; + int ai_protocol; in_port_t port; char ifname[IFNAMSIZ]; }; blob - 4b2db2c1ab45af5408ec20a9ce4e350fa0877b05 file + gotwebd/parse.y --- gotwebd/parse.y +++ gotwebd/parse.y @@ -1033,8 +1033,10 @@ get_addrs(const char *hostname, const char *servname, return (-1); } } - h->ss.ss_family = res->ai_family; + h->ai_family = res->ai_family; + h->ai_socktype = res->ai_socktype; + h->ai_protocol = res->ai_protocol; memcpy(&h->ss, res->ai_addr, res->ai_addrlen); h->slen = res->ai_addrlen; @@ -1068,7 +1070,10 @@ addr_dup_check(struct addresslist *al, struct address const char *addrstr; TAILQ_FOREACH(a, al, entry) { - if (a->slen != h->slen || + if (a->ai_family != h->ai_family || + a->ai_socktype != h->ai_socktype || + a->ai_protocol != h->ai_protocol || + a->slen != h->slen || memcmp(&a->ss, &h->ss, a->slen) != 0) continue; blob - dedacef4dfcf53fb027cced30494bbc5781d81c3 file + gotwebd/sockets.c --- gotwebd/sockets.c +++ gotwebd/sockets.c @@ -77,7 +77,7 @@ static void sockets_rlimit(int); static int sockets_dispatch_gotwebd(int, struct privsep_proc *, struct imsg *); static int sockets_unix_socket_listen(struct privsep *, struct socket *); -static int sockets_create_socket(struct address *, in_port_t); +static int sockets_create_socket(struct address *); static int sockets_accept_reserve(int, struct sockaddr *, socklen_t *, int, volatile int *); @@ -231,6 +231,9 @@ sockets_conf_new_socket_fcgi(struct gotwebd *env, stru memcpy(&acp->ss, &a->ss, sizeof(acp->ss)); acp->slen = a->slen; + acp->ai_family = a->ai_family; + acp->ai_socktype = a->ai_socktype; + acp->ai_protocol = a->ai_protocol; acp->port = a->port; if (*a->ifname != '\0') { if (strlcpy(acp->ifname, a->ifname, @@ -408,8 +411,7 @@ sockets_privinit(struct gotwebd *env, struct socket *s log_debug("%s: initializing %s FCGI socket on port %d for %s", __func__, sock->conf.af_type == AF_INET ? "inet" : "inet6", sock->conf.fcgi_socket_port, sock->conf.name); - sock->fd = sockets_create_socket(&sock->conf.addr, - sock->conf.fcgi_socket_port); + sock->fd = sockets_create_socket(&sock->conf.addr); if (sock->fd == -1) { log_warnx("%s: create FCGI socket failed", __func__); return -1; @@ -495,17 +497,11 @@ sockets_unix_socket_listen(struct privsep *ps, struct } static int -sockets_create_socket(struct address *a, in_port_t port) +sockets_create_socket(struct address *a) { - struct addrinfo hints; int fd = -1, o_val = 1, flags; - memset(&hints, 0, sizeof(hints)); - hints.ai_family = AF_UNSPEC; - hints.ai_socktype = SOCK_STREAM; - hints.ai_flags |= AI_PASSIVE; - - fd = socket(a->ss.ss_family, hints.ai_socktype, 0); + fd = socket(a->ai_family, a->ai_socktype, a->ai_protocol); if (fd == -1) return -1; @@ -530,8 +526,7 @@ sockets_create_socket(struct address *a, in_port_t por if (bind(fd, (struct sockaddr *)&a->ss, a->slen) == -1) { close(fd); - log_info("%s: can't bind to port %d", __func__, - ntohs(port)); + log_info("%s: can't bind to port %d", __func__, a->port); return -1; }