Download raw body.
gotwebd: merge host() and get_addrs(); use * instead of ""
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 <v.port> fcgiport
> %token <v.number> NUMBER
> %type <v.number> boolean
> +%type <v.string> 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);
> }
>
>
>
gotwebd: merge host() and get_addrs(); use * instead of ""