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 <syslog.h>
>  #include <unistd.h>
>  
> -#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 <stsp@openbsd.org>
> - *
> - * 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 <stsp@openbsd.org>
> - *
> - * 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 <sys/socket.h>
> -#include <netinet/in.h>
> -
> -#include <string.h>
> -
> -#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;
> -}
> 
>