[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 01/13] New virSocketAddr utility functions



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!


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]