Klemens Nanni wrote: > Besides fixing a tautological 'v < 0' and using more descriptive/less > errorprone sizeof(target) this patch unifies strtoul(3) handling both > logically and cosmetically.
A great deal of these look like exactly the situation strtonum was desinged to cope with. this means dropping support for hex bases, but i'm happy telling people they need to learn decimal. :) > > Index: brconfig.c > =================================================================== > RCS file: /cvs/src/sbin/ifconfig/brconfig.c,v > retrieving revision 1.14 > diff -u -p -r1.14 brconfig.c > --- brconfig.c 28 Nov 2016 10:12:50 -0000 1.14 > +++ brconfig.c 2 Jun 2017 15:55:29 -0000 > @@ -220,7 +220,6 @@ bridge_ifsetflag(const char *ifsname, u_ > err(1, "%s: ioctl SIOCBRDGGIFFLGS %s", name, ifsname); > > req.ifbr_ifsflags |= flag & ~IFBIF_RO_MASK; > - > if (ioctl(s, SIOCBRDGSIFFLGS, (caddr_t)&req) < 0) > err(1, "%s: ioctl SIOCBRDGSIFFLGS %s", name, ifsname); > } > @@ -232,12 +231,10 @@ bridge_ifclrflag(const char *ifsname, u_ > > strlcpy(req.ifbr_name, name, sizeof(req.ifbr_name)); > strlcpy(req.ifbr_ifsname, ifsname, sizeof(req.ifbr_ifsname)); > - > if (ioctl(s, SIOCBRDGGIFFLGS, (caddr_t)&req) < 0) > err(1, "%s: ioctl SIOCBRDGGIFFLGS %s", name, ifsname); > > req.ifbr_ifsflags &= ~(flag | IFBIF_RO_MASK); > - > if (ioctl(s, SIOCBRDGSIFFLGS, (caddr_t)&req) < 0) > err(1, "%s: ioctl SIOCBRDGSIFFLGS %s", name, ifsname); > } > @@ -425,7 +422,8 @@ bridge_maxage(const char *arg, int d) > > errno = 0; > v = strtoul(arg, &endptr, 0); > - if (arg[0] == '\0' || endptr[0] != '\0' || v > 0xffUL || > + if (arg[0] == '\0' || endptr[0] != '\0' || > + v == 0 || v > sizeof(bp.ifbrp_maxage) || > (errno == ERANGE && v == ULONG_MAX)) > errx(1, "invalid arg for maxage: %s", arg); > > @@ -444,7 +442,8 @@ bridge_priority(const char *arg, int d) > > errno = 0; > v = strtoul(arg, &endptr, 0); > - if (arg[0] == '\0' || endptr[0] != '\0' || v > 0xffffUL || > + if (arg[0] == '\0' || endptr[0] != '\0' || > + v == 0 || v > sizeof(bp.ifbrp_prio) || > (errno == ERANGE && v == ULONG_MAX)) > errx(1, "invalid arg for spanpriority: %s", arg); > > @@ -483,7 +482,8 @@ bridge_fwddelay(const char *arg, int d) > > errno = 0; > v = strtoul(arg, &endptr, 0); > - if (arg[0] == '\0' || endptr[0] != '\0' || v > 0xffUL || > + if (arg[0] == '\0' || endptr[0] != '\0' || > + v == 0 || v > sizeof(bp.ifbrp_fwddelay) || > (errno == ERANGE && v == ULONG_MAX)) > errx(1, "invalid arg for fwddelay: %s", arg); > > @@ -502,7 +502,8 @@ bridge_hellotime(const char *arg, int d) > > errno = 0; > v = strtoul(arg, &endptr, 0); > - if (arg[0] == '\0' || endptr[0] != '\0' || v > 0xffUL || > + if (arg[0] == '\0' || endptr[0] != '\0' || > + v == 0 || v > sizeof(bp.ifbrp_hellotime) || > (errno == ERANGE && v == ULONG_MAX)) > errx(1, "invalid arg for hellotime: %s", arg); > > @@ -521,7 +522,8 @@ bridge_maxaddr(const char *arg, int d) > > errno = 0; > newsize = strtoul(arg, &endptr, 0); > - if (arg[0] == '\0' || endptr[0] != '\0' || newsize > 0xffffffffUL || > + if (arg[0] == '\0' || endptr[0] != '\0' || > + newsize == 0 || newsize > sizeof(bp.ifbrp_csize) || > (errno == ERANGE && newsize == ULONG_MAX)) > errx(1, "invalid arg for maxaddr: %s", arg); > > @@ -537,13 +539,12 @@ bridge_deladdr(const char *addr, int d) > struct ifbareq ifba; > struct ether_addr *ea; > > - strlcpy(ifba.ifba_name, name, sizeof(ifba.ifba_name)); > ea = ether_aton(addr); > if (ea == NULL) > err(1, "Invalid address: %s", addr); > > + strlcpy(ifba.ifba_name, name, sizeof(ifba.ifba_name)); > bcopy(ea, &ifba.ifba_dst, sizeof(struct ether_addr)); > - > if (ioctl(s, SIOCBRDGDADDR, &ifba) < 0) > err(1, "%s: %s", name, addr); > } > @@ -555,16 +556,16 @@ bridge_ifprio(const char *ifname, const > unsigned long v; > char *endptr; > > - strlcpy(breq.ifbr_name, name, sizeof(breq.ifbr_name)); > - strlcpy(breq.ifbr_ifsname, ifname, sizeof(breq.ifbr_ifsname)); > - > errno = 0; > v = strtoul(val, &endptr, 0); > - if (val[0] == '\0' || endptr[0] != '\0' || v > 0xffUL || > + if (val[0] == '\0' || endptr[0] != '\0' || > + v == 0 || v > sizeof(breq.ifbr_priority) || > (errno == ERANGE && v == ULONG_MAX)) > - err(1, "invalid arg for ifpriority: %s", val); > - breq.ifbr_priority = v; > + errx(1, "invalid arg for ifpriority: %s", val); > > + strlcpy(breq.ifbr_name, name, sizeof(breq.ifbr_name)); > + strlcpy(breq.ifbr_ifsname, ifname, sizeof(breq.ifbr_ifsname)); > + breq.ifbr_priority = v; > if (ioctl(s, SIOCBRDGSIFPRIO, (caddr_t)&breq) < 0) > err(1, "%s: %s", name, val); > } > @@ -576,18 +577,16 @@ bridge_ifcost(const char *ifname, const > unsigned long v; > char *endptr; > > - strlcpy(breq.ifbr_name, name, sizeof(breq.ifbr_name)); > - strlcpy(breq.ifbr_ifsname, ifname, sizeof(breq.ifbr_ifsname)); > - > errno = 0; > v = strtoul(val, &endptr, 0); > if (val[0] == '\0' || endptr[0] != '\0' || > - v < 0 || v > 0xffffffffUL || > + v == 0 || v > sizeof(breq.ifbr_path_cost) || > (errno == ERANGE && v == ULONG_MAX)) > errx(1, "invalid arg for ifcost: %s", val); > > + strlcpy(breq.ifbr_name, name, sizeof(breq.ifbr_name)); > + strlcpy(breq.ifbr_ifsname, ifname, sizeof(breq.ifbr_ifsname)); > breq.ifbr_path_cost = v; > - > if (ioctl(s, SIOCBRDGSIFCOST, (caddr_t)&breq) < 0) > err(1, "%s: %s", name, val); > } > @@ -599,9 +598,7 @@ bridge_noifcost(const char *ifname, int > > strlcpy(breq.ifbr_name, name, sizeof(breq.ifbr_name)); > strlcpy(breq.ifbr_ifsname, ifname, sizeof(breq.ifbr_ifsname)); > - > breq.ifbr_path_cost = 0; > - > if (ioctl(s, SIOCBRDGSIFCOST, (caddr_t)&breq) < 0) > err(1, "%s", name); > } > @@ -612,16 +609,14 @@ bridge_addaddr(const char *ifname, const > struct ifbareq ifba; > struct ether_addr *ea; > > - strlcpy(ifba.ifba_name, name, sizeof(ifba.ifba_name)); > - strlcpy(ifba.ifba_ifsname, ifname, sizeof(ifba.ifba_ifsname)); > - > ea = ether_aton(addr); > if (ea == NULL) > errx(1, "Invalid address: %s", addr); > > + strlcpy(ifba.ifba_name, name, sizeof(ifba.ifba_name)); > + strlcpy(ifba.ifba_ifsname, ifname, sizeof(ifba.ifba_ifsname)); > bcopy(ea, &ifba.ifba_dst, sizeof(struct ether_addr)); > ifba.ifba_flags = IFBAF_STATIC; > - > if (ioctl(s, SIOCBRDGSADDR, &ifba) < 0) > err(1, "%s: %s", name, addr); > } > @@ -701,7 +696,6 @@ is_bridge(char *brdg) > struct ifbaconf ifbac; > > strlcpy(ifr.ifr_name, brdg, sizeof(ifr.ifr_name)); > - > if (ioctl(s, SIOCGIFFLAGS, (caddr_t)&ifr) < 0) > return (0); > > @@ -1052,7 +1046,9 @@ switch_datapathid(const char *arg, int d > > errno = 0; > newdpid = strtoull(arg, &endptr, 0); > - if (arg[0] == '\0' || endptr[0] != '\0' || errno == ERANGE) > + if (arg[0] == '\0' || endptr[0] != '\0' || > + newdpid == 0 || newdpid > sizeof(bp.ifbrp_datapath) || > + (errno == ERANGE && newdpid == ULLONG_MAX)) > errx(1, "invalid arg for datapath-id: %s", arg); > > strlcpy(bp.ifbrp_name, name, sizeof(bp.ifbrp_name)); > @@ -1068,14 +1064,13 @@ switch_portno(const char *ifname, const > uint32_t newportidx; > char *endptr; > > - strlcpy(breq.ifbr_name, name, sizeof(breq.ifbr_name)); > - strlcpy(breq.ifbr_ifsname, ifname, sizeof(breq.ifbr_ifsname)); > - > errno = 0; > newportidx = strtol(val, &endptr, 0); > if (val[0] == '\0' || endptr[0] != '\0' || errno == ERANGE) > errx(1, "invalid arg for portidx: %s", val); > > + strlcpy(breq.ifbr_name, name, sizeof(breq.ifbr_name)); > + strlcpy(breq.ifbr_ifsname, ifname, sizeof(breq.ifbr_ifsname)); > breq.ifbr_portno = newportidx; > if (ioctl(s, SIOCSWSPORTNO, (caddr_t)&breq) < 0) > err(1, "%s", name); >
