From: Stefan Sperling Subject: Re: gotwebd: merge host() and get_addrs(); use * instead of "" To: Omar Polo Cc: gameoftrees@openbsd.org Date: Wed, 15 Nov 2023 13:46:56 +0100 On Wed, Nov 15, 2023 at 10:25:52AM +0100, Omar Polo wrote: > This is a series of diffs that cleans up of we use getaddrinfo(), how we > reach into struct sockaddr_storage and also in preparation for future > semplifications. For the moment I'm happy to delete got_sockaddr.[ch], > but that's a followup, this and another step needs to be taken before. > > I prefer to split these steps into a few more manegeable diffs since it's > a bit intricate the relations between the various parts. > > This merges get_addrs() and host() and slightly improves our > getaddrinfo() usage. We currently use getaddrinfo() to resolve > hostnames and ip addresses but not services/port names, and then reach > into the struct storage. Instead: > > - pass the hostname or NULL when we want to listen on everything > > before we used "" to represent any address (see the deleted chunk at > the end). Here, I've changed the syntax of the config file use * > instead (only real change in the series of diffs). I'm not sure how > useful is to listen on any address, but that was the previous > behaviour and I kept it. Happy to drop it in a follow-up. > > - set AI_PASSIVE > > needed since we later want to use the return for bind() > > - pass the portname too > > so that we don't need to reach into the struct sockaddr_storage, but > this will be exploited in a future diff. For the moment, I'm only > re-converting the port number to a string and passing that. Further > cleanup will be done as follow-up. > > - removed GOTWEBD_MAXIFACE since it was only used in host(). > > ok? Ok by me, just some nits below: > diff /home/op/w/gotwebd > commit - af09dd3fec1090c7273f680a0ae5bf50d2183390 > path + /home/op/w/gotwebd > blob - 1ea7f7b6e6083db17472436ab39cbdb63fb3dc1f > file + gotwebd/gotwebd.h > --- gotwebd/gotwebd.h > +++ gotwebd/gotwebd.h > @@ -47,7 +47,6 @@ > #define GOTWEBD_MAXNAME 64 > #define GOTWEBD_MAXPORT 6 > #define GOTWEBD_NUMPROC 3 > -#define GOTWEBD_MAXIFACE 16 > #define GOTWEBD_REPO_CACHESIZE 4 > > /* GOTWEB DEFAULTS */ > blob - 300d47b9f6f477f3d3c2f3f874ceb2d817ce2425 > file + gotwebd/parse.y > --- gotwebd/parse.y > +++ gotwebd/parse.y > @@ -99,8 +99,6 @@ int get_addrs(const char *, struct server *, in_port > int addr_dup_check(struct addresslist *, struct address *, > const char *, const char *); > int add_addr(struct server *, struct address *); > -int host(const char *, struct server *, > - int, in_port_t, const char *); > > typedef struct { > union { > @@ -123,6 +121,7 @@ typedef struct { > %type fcgiport > %token NUMBER > %type boolean > +%type listen_addr > > %% > > @@ -176,6 +175,10 @@ boolean : STRING { > } > ; > > +listen_addr : '*' { $$ = NULL; } > + | STRING > + ; > + > fcgiport : PORT NUMBER { > if ($2 <= 0 || $2 > (int)USHRT_MAX) { > yyerror("invalid port: %lld", $2); > @@ -342,7 +345,7 @@ serveropts1 : REPOS_PATH STRING { > } > free($2); > } > - | LISTEN ON STRING fcgiport { > + | LISTEN ON listen_addr fcgiport { > if (get_addrs($3, new_srv, $4) == -1) { > yyerror("could not get addrs"); > YYERROR; > @@ -1020,39 +1023,40 @@ getservice(const char *n) > } > > int > -host(const char *s, struct server *new_srv, int max, > - in_port_t port, const char *ifname) > +get_addrs(const char *s, struct server *new_srv, in_port_t port) > { > struct addrinfo hints, *res0, *res; > - int error, cnt = 0; > + int n, error; > struct sockaddr_in *sain; > struct sockaddr_in6 *sin6; > struct address *h; > + char portstr[32]; > > + n = snprintf(portstr, sizeof(portstr), "%d", port); > + if (n < 0 || (size_t)n >= sizeof(portstr)) > + fatalx("snprintf: number too long"); This error message could provide more context to be helpful: fatalx("snprintf: port number too long: %d", port); > + > memset(&hints, 0, sizeof(hints)); > hints.ai_family = AF_UNSPEC; > hints.ai_socktype = SOCK_STREAM; /* DUMMY */ > - hints.ai_flags = AI_ADDRCONFIG; > - error = getaddrinfo(s, NULL, &hints, &res0); > - if (error == EAI_AGAIN || error == EAI_NODATA || error == EAI_NONAME) > - return (0); > + hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG; > + error = getaddrinfo(s, portstr, &hints, &res0); > if (error) { > log_warnx("%s: could not parse \"%s\": %s", __func__, s, > gai_strerror(error)); > return (-1); > } > > - for (res = res0; res && cnt < max; res = res->ai_next) { > - if (res->ai_family != AF_INET && > - res->ai_family != AF_INET6) > - continue; > + for (res = res0; res; res = res->ai_next) { > if ((h = calloc(1, sizeof(*h))) == NULL) > fatal(__func__); > > if (port) > h->port = port; > - if (ifname != NULL) { > - if (strlcpy(h->ifname, ifname, sizeof(h->ifname)) >= > + if (s == NULL) { > + strlcpy(h->ifname, "*", sizeof(h->ifname)); > + } else { > + if (strlcpy(h->ifname, s, sizeof(h->ifname)) >= > sizeof(h->ifname)) { > log_warnx("%s: interface name truncated", Change 'interface name' to 'address' in this warning? > __func__); > @@ -1077,35 +1081,8 @@ host(const char *s, struct server *new_srv, int max, > > if (add_addr(new_srv, h)) > return -1; > - cnt++; > } > - if (cnt == max && res) { > - log_warnx("%s: %s resolves to more than %d hosts", __func__, > - s, max); > - } > freeaddrinfo(res0); > - return (cnt); > -} > - > -int > -get_addrs(const char *addr, struct server *new_srv, in_port_t port) > -{ > - if (strcmp("", addr) == 0) { > - if (host("127.0.0.1", new_srv, 1, port, "127.0.0.1") <= 0) { > - yyerror("invalid listen ip: %s", > - "127.0.0.1"); > - return (-1); > - } > - if (host("::1", new_srv, 1, port, "::1") <= 0) { > - yyerror("invalid listen ip: %s", "::1"); > - return (-1); > - } > - } else { > - if (host(addr, new_srv, GOTWEBD_MAXIFACE, port, addr) <= 0) { > - yyerror("invalid listen ip: %s", addr); > - return (-1); > - } > - } > return (0); > } > > >