On Sat, Oct 20, 2018 at 11:57:13AM +0200, Denis Fondras wrote:
> Sync changes to host_*() from ntpd to relayd.
This looks good, however I'm not a relayd user.
How did you test it? With both IPv4 and IPv6?
Some nits inline to squash inconsistencies with other `host_ip()' users
in base.
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
> retrieving revision 1.228
> diff -u -p -r1.228 parse.y
> --- parse.y 7 Sep 2018 07:35:31 -0000 1.228
> +++ parse.y 20 Oct 2018 09:56:28 -0000
> @@ -123,8 +123,7 @@ static enum direction dir = RELAY_DIR_A
> static char *rulefile = NULL;
> static union hashkey *hashkey = NULL;
>
> -struct address *host_v4(const char *);
> -struct address *host_v6(const char *);
> +struct address *host_ip(const char *);
> int host_dns(const char *, struct addresslist *,
> int, struct portrange *, const char *, int);
> int host_if(const char *, struct addresslist *,
> @@ -2929,49 +2928,21 @@ symget(const char *nam)
> }
>
> struct address *
> -host_v4(const char *s)
> +host_ip(const char *s)
> {
> - struct in_addr ina;
> - struct sockaddr_in *sain;
> - struct address *h;
> -
> - bzero(&ina, sizeof(ina));
> - if (inet_pton(AF_INET, s, &ina) != 1)
> - return (NULL);
> -
> - if ((h = calloc(1, sizeof(*h))) == NULL)
> - fatal(__func__);
> - sain = (struct sockaddr_in *)&h->ss;
> - sain->sin_len = sizeof(struct sockaddr_in);
> - sain->sin_family = AF_INET;
> - sain->sin_addr.s_addr = ina.s_addr;
> -
> - return (h);
> -}
> -
> -struct address *
> -host_v6(const char *s)
> -{
> - struct addrinfo hints, *res;
> - struct sockaddr_in6 *sa_in6;
> + struct addrinfo hints, *res;
Please aline `hints' with a space after the tab.
> struct address *h = NULL;
>
> - bzero(&hints, sizeof(hints));
> - hints.ai_family = AF_INET6;
> - hints.ai_socktype = SOCK_DGRAM; /* dummy */
> + memset(&hints, 0, sizeof(hints));
> + hints.ai_family = AF_UNSPEC;
> + hints.ai_socktype = SOCK_DGRAM; /*dummy*/
> hints.ai_flags = AI_NUMERICHOST;
> if (getaddrinfo(s, "0", &hints, &res) == 0) {
> - if ((h = calloc(1, sizeof(*h))) == NULL)
> - fatal(__func__);
> - sa_in6 = (struct sockaddr_in6 *)&h->ss;
> - sa_in6->sin6_len = sizeof(struct sockaddr_in6);
> - sa_in6->sin6_family = AF_INET6;
> - memcpy(&sa_in6->sin6_addr,
> - &((struct sockaddr_in6 *)res->ai_addr)->sin6_addr,
> - sizeof(sa_in6->sin6_addr));
> - sa_in6->sin6_scope_id =
> - ((struct sockaddr_in6 *)res->ai_addr)->sin6_scope_id;
> -
> + if (res->ai_family == AF_INET || res->ai_family == AF_INET6) {
In ntpd and pfctl we break this into two lines, mind doing so here, too?
> + if ((h = calloc(1, sizeof(*h))) == NULL)
> + fatal(NULL);
> + memcpy(&h->ss, res->ai_addr, res->ai_addrlen);
> + }
> freeaddrinfo(res);
> }
>
> @@ -2984,15 +2955,13 @@ host_dns(const char *s, struct addressli
> {
> struct addrinfo hints, *res0, *res;
> int error, cnt = 0;
> - struct sockaddr_in *sain;
> - struct sockaddr_in6 *sin6;
> struct address *h;
>
> if ((cnt = host_if(s, al, max, port, ifname, ipproto)) != 0)
> return (cnt);
>
> bzero(&hints, sizeof(hints));
> - hints.ai_family = PF_UNSPEC;
> + hints.ai_family = AF_UNSPEC;
> hints.ai_socktype = SOCK_DGRAM; /* DUMMY */
> hints.ai_flags = AI_ADDRCONFIG;
> error = getaddrinfo(s, NULL, &hints, &res0);
> @@ -3024,19 +2993,8 @@ host_dns(const char *s, struct addressli
> }
> if (ipproto != -1)
> h->ipproto = ipproto;
> - h->ss.ss_family = res->ai_family;
>
> - if (res->ai_family == AF_INET) {
> - sain = (struct sockaddr_in *)&h->ss;
> - sain->sin_len = sizeof(struct sockaddr_in);
> - sain->sin_addr.s_addr = ((struct sockaddr_in *)
> - res->ai_addr)->sin_addr.s_addr;
> - } else {
> - sin6 = (struct sockaddr_in6 *)&h->ss;
> - sin6->sin6_len = sizeof(struct sockaddr_in6);
> - memcpy(&sin6->sin6_addr, &((struct sockaddr_in6 *)
> - res->ai_addr)->sin6_addr, sizeof(struct in6_addr));
> - }
> + memcpy(&h->ss, res->ai_addr, res->ai_addrlen);
>
> TAILQ_INSERT_HEAD(al, h, entry);
> cnt++;
> @@ -3125,11 +3083,7 @@ host(const char *s, struct addresslist *
> {
> struct address *h;
Replace this space with a tab, please.
> - h = host_v4(s);
> -
> - /* IPv6 address? */
> - if (h == NULL)
> - h = host_v6(s);
> + h = host_ip(s);
>
> if (h != NULL) {
> if (port != NULL)
`if ((h = host_ip(s)) == NULL)' is used in ntpd and pfctl, I'd like to
keep it in sync here as well.
