Re: [libvirt] [PATCH 01/13] New virSocketAddr utility functions
On Mon, Dec 20, 2010 at 09:03, Laine Stump <laine(a)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.
+/**
+ * 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?
--
Pawel