On 12/20/2010 05:45 PM, Paweł Krześniak wrote:
On Mon, Dec 20, 2010 at 09:03, Laine Stump<laine@laine.org> wrote:
diff --git a/src/util/network.c b/src/util/network.c index 1abe78b..e4791b9 100644 --- a/src/util/network.c +++ b/src/util/network.c @@ -288,6 +288,73 @@ int virSocketAddrIsNetmask(virSocketAddrPtr netmask) { }
/** + * virSocketAddrMask: + * @addr: address that needs to be masked + * @netmask: the netmask address + * + * Mask off the host bits of @addr according to @netmask, turning it + * into a network address. + * + * Returns 0 in case of success, or -1 on error. + */ +int +virSocketAddrMask(virSocketAddrPtr addr, const virSocketAddrPtr netmask) +{ + if (addr->data.stor.ss_family != netmask->data.stor.ss_family) + return -1; + + if (addr->data.stor.ss_family == AF_INET) { + addr->data.inet4.sin_addr.s_addr +&= netmask->data.inet4.sin_addr.s_addr; + return 0; + } + if (addr->data.stor.ss_family == AF_INET6) { + int ii; + for (ii = 0; ii< 4; ii++) + addr->data.inet6.sin6_addr.s6_addr32[ii] +&= netmask->data.inet6.sin6_addr.s6_addr32[ii]; + return 0; + } + return -1; +} + +/** + * virSocketAddrMaskByPrefix: + * @addr: address that needs to be masked + * @prefix: prefix (# of 1 bits) of netmask to apply + * + * Mask off the host bits of @addr according to @prefix, turning it + * into a network address. + * + * Returns 0 in case of success, or -1 on error. + */ +int +virSocketAddrMaskByPrefix(virSocketAddrPtr addr, int prefix) +{ + virSocketAddr netmask; + + if (virSocketAddrPrefixToNetmask(prefix,&netmask, + addr->data.stor.ss_family)< 0) + return -1; why not replace following:
+ if (addr->data.stor.ss_family == AF_INET) { + + addr->data.inet4.sin_addr.s_addr +&= netmask.data.inet4.sin_addr.s_addr; + return 0; + } + if (addr->data.stor.ss_family == AF_INET6) { + int ii; + + for (ii = 0; ii< 4; ii++) + addr->data.inet6.sin6_addr.s6_addr32[ii] +&= netmask.data.inet6.sin6_addr.s6_addr32[ii]; + return 0; + } + return -1; +} with simple: return virSocketAddrMask(addr, netmask);
With cost of checking ss_family for addr and netmask we achieve clearer code.
Right. Eric also suggested this, and I can't even tell you why I didn't do it that way to begin with. :-P
+/** + * virSocketPrefixToNetmask: + * @prefix: number of 1 bits to put in the netmask + * @netmask: address to fill in with the desired netmask + * @family: family of the address (AF_INET or AF_INET6 only) + * + * given @prefix and @family, fill in @netmask with a netmask + * (eg 255.255.255.0). + * + * Returns 0 on success or -1 on error. + */ + +int virSocketAddrPrefixToNetmask(int prefix, + virSocketAddrPtr netmask, + int family) +{ + int result = -1; + + netmask->data.stor.ss_family = AF_UNSPEC; /* assume failure */ You don't check if (prefix< 0) for AF_INET6. So my suggestion is to check it here: if (prefix< 0) goto error;
+ if (family == AF_INET) { + int ip = 0; + and remove this check from here:
+ if (prefix< 0 || prefix> 32) + goto error; + btw: does prefix really needs to be signed int?
No. I had it that way for consistency with prefix return values (which are ints so we can return -1 for an error), but after your and Eric's comments, I'm changing them to unsigned int (but leaving the return values as ints). v2 is coming up. Thanks for the comments!