On Sat, May 26, 2018 at 12:54:03PM +0200, Klemens Nanni wrote:
> inet6_makenetandmask() parses the provided prefix length `plen' twice:
> first from inside prefixlen() which sets the mask, then right again
> doing almost the same dance to find out which bits to zero in the
> provided address.
>
> But once prefixlen() set so_mask, we can just use it to actually mask
> the address.
>
> prefixlen() returns `(len == max)' where `max = 128' for `AF_INET6', so
> it's equivalent to the strcmp() call.
>
> Regress tests pass on amd64, also no issue after manually adding,
> testing and removing routes on my machine.
>
> Feedback? OK?
>
> Index: route.c
> ===================================================================
> RCS file: /cvs/src/sbin/route/route.c,v
> retrieving revision 1.214
> diff -u -p -r1.214 route.c
> --- route.c 1 May 2018 18:14:10 -0000 1.214
> +++ route.c 26 May 2018 10:49:50 -0000
> @@ -786,21 +786,17 @@ inet_makenetandmask(u_int32_t net, struc
> sin->sin_len = 1 + cp - (char *)sin;
> }
>
> -/*
> - * XXX the function may need more improvement...
> - */
> int
> inet6_makenetandmask(struct sockaddr_in6 *sin6, char *plen)
> {
> struct in6_addr in6;
> - const char *errstr;
> - int i, len, q, r;
> + int i;
>
> - if (NULL==plen) {
> + if (!plen) {
> if (IN6_IS_ADDR_UNSPECIFIED(&sin6->sin6_addr) &&
> - sin6->sin6_scope_id == 0) {
> + sin6->sin6_scope_id == 0)
> plen = "0";
> - } else if ((sin6->sin6_addr.s6_addr[0] & 0xe0) == 0x20) {
> + else if ((sin6->sin6_addr.s6_addr[0] & 0xe0) == 0x20) {
> /* aggregatable global unicast - RFC2374 */
> memset(&in6, 0, sizeof(in6));
> if (!memcmp(&sin6->sin6_addr.s6_addr[8],
Not related to this diff but RFC2374 has been made obsolete by RFC3587 for some
years : "implementations should not make any assumptions about 2000::/3 being
special". I think we can simplify this "else if" :)
> @@ -809,27 +805,14 @@ inet6_makenetandmask(struct sockaddr_in6
> }
> }
>
> - if (!plen || strcmp(plen, "128") == 0)
> + if (!plen || prefixlen(AF_INET6, plen))
> return (1);
> - else {
> - rtm_addrs |= RTA_NETMASK;
> - prefixlen(AF_INET6, plen);
> -
> - len = strtonum(plen, 0, 128, &errstr);
> - if (errstr)
> - errx(1, "prefixlen %s is %s", plen, errstr);
> -
> - q = (128-len) >> 3;
> - r = (128-len) & 7;
> - i = 15;
> -
> - while (q-- > 0)
> - sin6->sin6_addr.s6_addr[i--] = 0;
> - if (r > 0)
> - sin6->sin6_addr.s6_addr[i] &= 0xff << r;
> + rtm_addrs |= RTA_NETMASK;
Can't we consider this done in prefixlen() ?
>
> - return (0);
> - }
> + for (i = 0; i < 16; ++i)
> + sin6->sin6_addr.s6_addr[i] &= so_mask.sin6.sin6_addr.s6_addr[i];
> +
> + return (0);
> }
>
> /*
>
