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); > } > > /* >