"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: get rid of got_sockaddr.[ch]
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Wed, 15 Nov 2023 17:15:47 +0100

Download raw body.

Thread
On Wed, Nov 15, 2023 at 04:47:45PM +0100, Omar Polo wrote:
> 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 :/

It is still a simplification. The data is spread over too many places
in the current code, and I find your new code less confusing as a result.
ok.

> 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;
>  	}
>  
>