Move union sa_ss to sa.h

Fix sa_fromprefix so that zero'd netmask bytes do not overwrite the last bit
set.
If assert is being used, add tests that sa_fromprefix and sa_toprefix generate
correct results by agreeing with each other.

diff --git a/route.h b/route.h
index 0c7ecc07b817b7454310e9d98ce5ef46859279d3..1da393bd516f143ffee76222262ae471718363d7 100644 (file)
--- a/route.h
+++ b/route.h
@@ -34,6 +34,7 @@
 #include <stdbool.h>
 
 #include "dhcpcd.h"
+#include "sa.h"
 
 /* Some systems have route metrics.
  * OpenBSD route priority is not this. */
@@ -49,12 +50,6 @@
  * But that's generally OK if only dhcpcd is managing routes. */
 #endif
 
-union sa_ss {
-       struct sockaddr         sa;
-       struct sockaddr_in      sin;
-       struct sockaddr_in6     sin6;
-};
-
 struct rt {
        TAILQ_ENTRY(rt)         rt_next;
        union sa_ss             rt_ss_dest;

diff --git a/sa.c b/sa.c
index 609f9ee15de55fad4c7aec97759804f7e186c2b2..07e85d8f90126f3b6f85afbe2f819327ea4b3751 100644 (file)
--- a/sa.c
+++ b/sa.c
@@ -45,6 +45,10 @@
 #include "common.h"
 #include "sa.h"
 
+#ifndef NDEBUG
+static bool sa_inprefix;
+#endif
+
 socklen_t
 sa_addroffset(const struct sockaddr *sa)
 {
@@ -184,6 +188,7 @@ sa_is_loopback(const struct sockaddr *sa)
 int
 sa_toprefix(const struct sockaddr *sa)
 {
+       int prefix;
 
        assert(sa != NULL);
        switch(sa->sa_family) {
@@ -192,21 +197,22 @@ sa_toprefix(const struct sockaddr *sa)
        {
                const struct sockaddr_in *sin;
                int mask;
-               int cidr;
 
                sin = satocsin(sa);
-               if (sin->sin_addr.s_addr == INADDR_ANY)
-                       return 0;
+               if (sin->sin_addr.s_addr == INADDR_ANY) {
+                       prefix = 0;
+                       break;
+               }
                mask = (int)ntohl(sin->sin_addr.s_addr);
-               cidr = 33 - ffs(mask);          /* 33 - (1 .. 32) -> 32 .. 1 */
-               if (cidr < 32) {                /* more than 1 bit in mask */
+               prefix = 33 - ffs(mask);        /* 33 - (1 .. 32) -> 32 .. 1 */
+               if (prefix < 32) {              /* more than 1 bit in mask */
                        /* check for non-contig netmask */
-                       if ((mask ^ (((1 << cidr) - 1) << (32 - cidr))) != 0) {
+                       if ((mask ^ (((1 << prefix)-1) << (32 - prefix))) != 0){
                                errno = EINVAL;
                                return -1;      /* noncontig, no pfxlen */
                        }
                }
-               return cidr;
+               break;
        }
 #endif
 #ifdef INET6
@@ -243,20 +249,36 @@ sa_toprefix(const struct sockaddr *sa)
                                        return 0;
                }
 
-               return (uint8_t)(x * NBBY + y);
+               prefix = x * NBBY + y;
+               break;
        }
 #endif
        default:
                errno = EAFNOSUPPORT;
                return -1;
        }
+
+#ifndef NDEBUG
+       /* Ensure the calculation is correct */
+       if (!sa_inprefix) {
+               union sa_ss ss;
+
+               sa_inprefix = true;
+               ss.sa.sa_family = sa->sa_family;
+               sa_fromprefix(&ss.sa, prefix);
+               assert(sa_cmp(sa, &ss.sa) == 0);
+               sa_inprefix = false;
+       }
+#endif
+
+       return prefix;
 }
 
 int
 sa_fromprefix(struct sockaddr *sa, int prefix)
 {
-       uint8_t *ap, a;
-       int max_prefix, i;
+       uint8_t *ap;
+       int max_prefix, bytes, bits, i;
 
        switch (sa->sa_family) {
 #ifdef INET
@@ -274,14 +296,31 @@ sa_fromprefix(struct sockaddr *sa, int prefix)
                return -1;
        }
 
+       bytes = prefix / NBBY;
+       bits = prefix % NBBY;
+
        ap = (uint8_t *)sa + sa_addroffset(sa);
-       for (i = 0; i < (prefix / NBBY); i++)
+       for (i = 0; i < bytes; i++)
                *ap++ = 0xff;
-       a = 0xff;
-       a  = (uint8_t)(a << (8 - (prefix % NBBY)));
-       *ap = a;
-       for (i = 0; i < ((max_prefix - prefix) / NBBY); i++)
+       if (bits) {
+               uint8_t a;
+
+               a = 0xff;
+               a  = (uint8_t)(a << (8 - bits));
+               *ap++ = a;
+       }
+       bytes = (max_prefix - prefix) / NBBY;
+       for (i = 0; i < bytes; i++)
                *ap++ = 0x00;
+
+#ifndef NDEBUG
+       /* Ensure the calculation is correct */
+       if (!sa_inprefix) {
+               sa_inprefix = true;
+               assert(sa_toprefix(sa) == prefix);
+               sa_inprefix = false;
+       }
+#endif
        return 0;
 }
 

diff --git a/sa.h b/sa.h
index f958da29481e4642091c8a2cb106c453608d224e..c2795d9d6ad84a67a8b3a07018b938ee37693255 100644 (file)
--- a/sa.h
+++ b/sa.h
@@ -30,6 +30,12 @@
 
 #include <sys/socket.h>
 
+union sa_ss {
+       struct sockaddr         sa;
+       struct sockaddr_in      sin;
+       struct sockaddr_in6     sin6;
+};
+
 #ifndef __linux__
 #define HAVE_SA_LEN
 #endif