From: Stefan Sperling Subject: Re: get rid of got_sockaddr.[ch] To: Omar Polo Cc: gameoftrees@openbsd.org Date: Wed, 15 Nov 2023 14:48:34 +0100 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. OK by me. This is a nice simplification. > diff /home/op/w/gotwebd > commit - b8b20b3c52fa4f90a1ac5b20e7d8a24fae8d9e27 > path + /home/op/w/gotwebd > blob - ff4e4df2ca69c232fb996d70160f9de9e52d0347 > file + gotwebd/Makefile > --- gotwebd/Makefile > +++ gotwebd/Makefile > @@ -18,7 +18,7 @@ SRCS += blame.c commit_graph.c delta.c diff.c \ > gotconfig.c diff_main.c diff_atomize_text.c diff_myers.c \ > diff_output.c diff_output_plain.c diff_output_unidiff.c \ > diff_output_edscript.c diff_patience.c bloom.c murmurhash2.c \ > - sigs.c date.c sockaddr.c \ > + sigs.c date.c \ > object_open_privsep.c read_gitconfig_privsep.c \ > read_gotconfig_privsep.c pollfd.c reference_parse.c \ > object_qid.c > blob - 3aa096b3ae2f9aa0d09d4724ebd87802d2feec54 > file + gotwebd/gotwebd.h > --- gotwebd/gotwebd.h > +++ gotwebd/gotwebd.h > @@ -263,7 +263,7 @@ struct fcgi_end_request_body { > struct address { > TAILQ_ENTRY(address) entry; > struct sockaddr_storage ss; > - int ipproto; > + socklen_t slen; > in_port_t port; > char ifname[IFNAMSIZ]; > }; > blob - cf8c89590e35a8671f7a6587dc7aa75e7f2e6cd3 > file + gotwebd/parse.y > --- gotwebd/parse.y > +++ gotwebd/parse.y > @@ -47,7 +47,6 @@ > #include > #include > > -#include "got_sockaddr.h" > #include "got_reference.h" > > #include "proc.h" > @@ -1003,8 +1002,8 @@ get_addrs(const char *hostname, const char *servname, > { > struct addrinfo hints, *res0, *res; > int error; > - struct sockaddr_in *sain, *ra; > - struct sockaddr_in6 *sin6, *ra6; > + struct sockaddr_in *sin; > + struct sockaddr_in6 *sin6; > struct address *h; > > memset(&hints, 0, sizeof(hints)); > @@ -1036,18 +1035,17 @@ get_addrs(const char *hostname, const char *servname, > } > h->ss.ss_family = res->ai_family; > > + memcpy(&h->ss, res->ai_addr, res->ai_addrlen); > + h->slen = res->ai_addrlen; > + > switch (res->ai_family) { > case AF_INET: > - sain = (struct sockaddr_in *)&h->ss; > - ra = (struct sockaddr_in *)res->ai_addr; > - h->port = ntohs(ra->sin_port); > - got_sockaddr_inet_init(sain, &ra->sin_addr); > + sin = (struct sockaddr_in *)res->ai_addr; > + h->port = ntohs(sin->sin_port); > break; > case AF_INET6: > - sin6 = (struct sockaddr_in6 *)&h->ss; > - ra6 = (struct sockaddr_in6 *)res->ai_addr; > - h->port = ntohs(ra6->sin6_port); > - got_sockaddr_inet6_init(sin6, &ra6->sin6_addr, 0); > + sin6 = (struct sockaddr_in6 *)res->ai_addr; > + h->port = ntohs(sin6->sin6_port); > break; > default: > fatalx("unknown address family %d", res->ai_family); > @@ -1070,8 +1068,8 @@ addr_dup_check(struct addresslist *al, struct address > const char *addrstr; > > TAILQ_FOREACH(a, al, entry) { > - if (memcmp(&a->ss, &h->ss, sizeof(h->ss)) != 0 || > - a->port != h->port) > + if (a->slen != h->slen || > + memcmp(&a->ss, &h->ss, a->slen) != 0) > continue; > > switch (h->ss.ss_family) { > blob - 62a293eeac4877b0dbd3ba7efad3968b35396de3 > file + gotwebd/sockets.c > --- gotwebd/sockets.c > +++ gotwebd/sockets.c > @@ -230,7 +230,7 @@ sockets_conf_new_socket_fcgi(struct gotwebd *env, stru > acp = &sock->conf.addr; > > memcpy(&acp->ss, &a->ss, sizeof(acp->ss)); > - acp->ipproto = a->ipproto; > + acp->slen = a->slen; > acp->port = a->port; > if (*a->ifname != '\0') { > if (strlcpy(acp->ifname, a->ifname, > @@ -505,19 +505,7 @@ sockets_create_socket(struct address *a, in_port_t por > hints.ai_socktype = SOCK_STREAM; > hints.ai_flags |= AI_PASSIVE; > > - switch (a->ss.ss_family) { > - case AF_INET: > - ((struct sockaddr_in *)(&a->ss))->sin_port = htons(port); > - break; > - case AF_INET6: > - ((struct sockaddr_in6 *)(&a->ss))->sin6_port = htons(port); > - break; > - default: > - log_warnx("%s: unknown address family", __func__); > - return -1; > - } > - > - fd = socket(a->ss.ss_family, hints.ai_socktype, a->ipproto); > + fd = socket(a->ss.ss_family, hints.ai_socktype, 0); > if (fd == -1) > return -1; > > @@ -540,7 +528,7 @@ sockets_create_socket(struct address *a, in_port_t por > return -1; > } > > - if (bind(fd, (struct sockaddr *)&a->ss, a->ss.ss_len) == -1) { > + if (bind(fd, (struct sockaddr *)&a->ss, a->slen) == -1) { > close(fd); > log_info("%s: can't bind to port %d", __func__, > ntohs(port)); > blob - 809c5c9b65985619eabf4ad948d80348f606762c > file + /dev/null > --- include/got_sockaddr.h > +++ /dev/null > @@ -1,19 +0,0 @@ > -/* > - * Copyright (c) 2022 Stefan Sperling > - * > - * Permission to use, copy, modify, and distribute this software for any > - * purpose with or without fee is hereby granted, provided that the above > - * copyright notice and this permission notice appear in all copies. > - * > - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > - * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > - * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > - */ > - > -void got_sockaddr_inet_init(struct sockaddr_in *in, struct in_addr *ina); > -void got_sockaddr_inet6_init(struct sockaddr_in6 *in6, struct in6_addr *in6a, > - uint32_t sin6_scope_id); > blob - 7a76593a59f98468b3891e3823c6d7fd0cfa960d > file + /dev/null > --- lib/sockaddr.c > +++ /dev/null > @@ -1,46 +0,0 @@ > -/* > - * Copyright (c) 2022 Stefan Sperling > - * > - * Permission to use, copy, modify, and distribute this software for any > - * purpose with or without fee is hereby granted, provided that the above > - * copyright notice and this permission notice appear in all copies. > - * > - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > - * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > - * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > - */ > - > -#include > -#include > - > -#include > - > -#include "got_sockaddr.h" > - > -/* > - * These interfaces wrap BSD-specific internals of internet address > - * data structures in a single compilation unit, allowing got-portable > - * to override them as needed, without a need for #ifdef macros. > - */ > - > -void > -got_sockaddr_inet_init(struct sockaddr_in *in, struct in_addr *ina) > -{ > - in->sin_len = sizeof(struct sockaddr_in); /* BSD-specific */ > - in->sin_family = AF_INET; > - in->sin_addr.s_addr = ina->s_addr; > -} > - > -void > -got_sockaddr_inet6_init(struct sockaddr_in6 *in6, struct in6_addr *in6a, > - uint32_t sin6_scope_id) > -{ > - in6->sin6_len = sizeof(struct sockaddr_in6); /* BSD-specific */ > - in6->sin6_family = AF_INET6; > - memcpy(&in6->sin6_addr, in6a, sizeof(in6->sin6_addr)); > - in6->sin6_scope_id = sin6_scope_id; > -} > >