Casper.Dik at Sun.COM wrote: >> Darren Reed writes: >> >>> As part of this continuing code review, I looked at >>> http://cr.opensolaris.org/~tonyn/firewall13Jan2009/usr/src/cmd/svc/servinfo/servinfo.c >>> >>> and I'm wondering if you have, in your latest workspace, a version >>> of uaddr2port() that doesn't leak memory from the malloc() for port_str? >>> The other alternative is to use alloca() instead. >>> >> I doubt that either malloc or alloca is needed here. strtol should be >> able to crack those numbers without having to make a separate copy. >> > > > What is the code doing? > > 79 if ((dot = strrchr(addr, '.')) == 0) { > 80 return (0); > 81 } else { > 82 char *p = dot - 1; > 83 size_t l = 0; > 84 while (*p != '.') { > 85 p--; > 86 l++; > 87 } > 88 p++; > 89 > 90 bzero(port_str, addr_len); > 91 (void) strncpy(port_str, p, l); > 92 port = atol(port_str) << 8; > 93 > 94 (void) strlcpy(port_str, dot + 1, addr_len); > 95 port = port | atol(port_str); > 96 } > 97 > 98 return (port); > > And are we sure that the code is only called with an address with 2 dots? > > I agree that strtol is probably the best function to use here. > > > p = addr + strlen(addr); > > dots = 0; > > while (p-- > addr) > if (*p == '.' && ++dots == 2) > break; > > if (dots != 2) > return (0); > p++; > port = strtoul(p, &q, 10) << 8; > if (q == p || q != '.') > return (0); > port |= strtoul(q + 1, NULL, 10); > return (port); >
It is looking for something like: 10.1.1.1.10.20 To mean that IP#10.1.1.1 is using port 2580 But it is also trying to pick ports off the end of an IPv6 address too...ie fe80:1:2:3:4:5:6:7.10.20 Darren
