chiark / gitweb /
comm etc.: Provide comm_addr_equal
authorIan Jackson <ijackson@chiark.greenend.org.uk>
Sun, 5 Oct 2014 11:03:21 +0000 (12:03 +0100)
committerIan Jackson <ijackson@chiark.greenend.org.uk>
Sun, 5 Oct 2014 20:27:28 +0000 (21:27 +0100)
Abolish the rule that a comm_addr has zeroes in all its holes.
Provide comm_addr_equal instead.

We can get rid of a lot of calls to FILLZERO.

In resolver.c we no longer need to copy the fields of ia individually.
We still need to look at the incoming address family since util.c
aborts on unknown AFs and adns (perhaps a new version or something)
might have sent us things we don't understand.  (Also reorganise the
loop/switch a little to get `wslot++' out of the `case'.)

We have to move the declaration of iaddr_equal.

Abolish transport_addrs_equal and replace it at call sites with the
new comm_addr_equal.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
resolver.c
secnet.h
site.c
udp.c
util.h

index b8a7a34..d6bc619 100644 (file)
@@ -42,7 +42,6 @@ static bool_t resolve_request(void *sst, cstring_t name,
        memcpy(trimmed,name+1,l-2);
        trimmed[l-2]=0;
        struct comm_addr ca;
        memcpy(trimmed,name+1,l-2);
        trimmed[l-2]=0;
        struct comm_addr ca;
-       FILLZERO(ca);
        ca.comm=comm;
        ca.ia.sin.sin_family=AF_INET;
        ca.ia.sin.sin_port=htons(port);
        ca.comm=comm;
        ca.ia.sin.sin_family=AF_INET;
        ca.ia.sin.sin_port=htons(port);
@@ -102,7 +101,6 @@ static void resolver_afterpoll(void *sst, struct pollfd *fds, int nfds)
                int rslot, wslot, total;
                int ca_len=MIN(ans->nrrs,MAX_PEER_ADDRS);
                struct comm_addr ca_buf[ca_len];
                int rslot, wslot, total;
                int ca_len=MIN(ans->nrrs,MAX_PEER_ADDRS);
                struct comm_addr ca_buf[ca_len];
-               FILLZERO(ca_buf);
                for (rslot=0, wslot=0, total=0;
                     rslot<ans->nrrs;
                     rslot++) {
                for (rslot=0, wslot=0, total=0;
                     rslot<ans->nrrs;
                     rslot++) {
@@ -111,18 +109,16 @@ static void resolver_afterpoll(void *sst, struct pollfd *fds, int nfds)
                    adns_rr_addr *ra=&ans->rrs.addr[rslot];
                    struct comm_addr *ca=&ca_buf[wslot];
                    ca->comm=q->comm;
                    adns_rr_addr *ra=&ans->rrs.addr[rslot];
                    struct comm_addr *ca=&ca_buf[wslot];
                    ca->comm=q->comm;
-                   /* copy fields individually so we leave holes zeroed: */
                    switch (ra->addr.sa.sa_family) {
                    case AF_INET:
                        assert(ra->len == sizeof(ca->ia.sin));
                    switch (ra->addr.sa.sa_family) {
                    case AF_INET:
                        assert(ra->len == sizeof(ca->ia.sin));
-                       ca->ia.sin.sin_family=ra->addr.inet.sin_family;
-                       ca->ia.sin.sin_addr=  ra->addr.inet.sin_addr;
-                       ca->ia.sin.sin_port=  htons(q->port);
-                       wslot++;
                        break;
                    default:
                        break;
                    default:
-                       break;
+                       /* silently skip unexpected AFs from adns */
+                       continue;
                    }
                    }
+                   memcpy(&ca->ia,&ra->addr,ra->len);
+                   wslot++;
                }
                q->answer(q->cst,ca_buf,wslot,total);
                free(q);
                }
                q->answer(q->cst,ca_buf,wslot,total);
                free(q);
index f458f8a..3fba00f 100644 (file)
--- a/secnet.h
+++ b/secnet.h
@@ -334,11 +334,6 @@ struct rsaprivkey_if {
 struct comm_addr {
     /* This struct is pure data; in particular comm's clients may
        freely copy it. */
 struct comm_addr {
     /* This struct is pure data; in particular comm's clients may
        freely copy it. */
-    /* Everyone is also guaranteed that all padding is set to zero, ie
-       that comm_addrs referring to semantically identical peers will
-       compare equal with memcmp.  Anyone who constructs a comm_addr
-       must start by memsetting it with FILLZERO, or some
-       equivalent. */
     struct comm_if *comm;
     union iaddr ia;
 };
     struct comm_if *comm;
     union iaddr ia;
 };
@@ -367,11 +362,19 @@ struct comm_if {
     comm_addr_to_string_fn *addr_to_string;
 };
 
     comm_addr_to_string_fn *addr_to_string;
 };
 
+bool_t iaddr_equal(const union iaddr *ia, const union iaddr *ib);
+
 static inline const char *comm_addr_to_string(const struct comm_addr *ca)
 {
     return ca->comm->addr_to_string(ca->comm->st, ca);
 }
 
 static inline const char *comm_addr_to_string(const struct comm_addr *ca)
 {
     return ca->comm->addr_to_string(ca->comm->st, ca);
 }
 
+static inline bool_t comm_addr_equal(const struct comm_addr *a,
+                                    const struct comm_addr *b)
+{
+    return a->comm==b->comm && iaddr_equal(&a->ia,&b->ia);
+}
+
 /* LOG interface */
 
 #define LOG_MESSAGE_BUFLEN 1023
 /* LOG interface */
 
 #define LOG_MESSAGE_BUFLEN 1023
diff --git a/site.c b/site.c
index d3eb662..d360699 100644 (file)
--- a/site.c
+++ b/site.c
@@ -2088,11 +2088,6 @@ static void transport_peers_debug(struct site *st, transport_peers *dst,
     }
 }
 
     }
 }
 
-static bool_t transport_addrs_equal(const struct comm_addr *a,
-                                   const struct comm_addr *b) {
-    return !memcmp(a,b,sizeof(*a));
-}
-
 static void transport_peers_expire(struct site *st, transport_peers *peers) {
     /* peers must be sorted first */
     int previous_peers=peers->npeers;
 static void transport_peers_expire(struct site *st, transport_peers *peers) {
     /* peers must be sorted first */
     int previous_peers=peers->npeers;
@@ -2116,7 +2111,7 @@ static bool_t transport_peer_record_one(struct site *st, transport_peers *peers,
        return 0;
 
     for (search=0; search<peers->npeers; search++)
        return 0;
 
     for (search=0; search<peers->npeers; search++)
-       if (transport_addrs_equal(&peers->peers[search].addr, ca))
+       if (comm_addr_equal(&peers->peers[search].addr, ca))
            return 1;
 
     peers->peers[peers->npeers].addr = *ca;
            return 1;
 
     peers->peers[peers->npeers].addr = *ca;
@@ -2135,7 +2130,7 @@ static void transport_record_peers(struct site *st, transport_peers *peers,
      * Caller must first call transport_peers_expire. */
 
     if (naddrs==1 && peers->npeers>=1 &&
      * Caller must first call transport_peers_expire. */
 
     if (naddrs==1 && peers->npeers>=1 &&
-       transport_addrs_equal(&addrs[0], &peers->peers[0].addr)) {
+       comm_addr_equal(&addrs[0], &peers->peers[0].addr)) {
        /* optimisation, also avoids debug for trivial updates */
        peers->peers[0].last = *tv_now;
        return;
        /* optimisation, also avoids debug for trivial updates */
        peers->peers[0].last = *tv_now;
        return;
diff --git a/udp.c b/udp.c
index 4216922..477dea3 100644 (file)
--- a/udp.c
+++ b/udp.c
@@ -83,7 +83,6 @@ static void udp_afterpoll(void *state, struct pollfd *fds, int nfds)
 
     if (nfds && (fds->revents & POLLIN)) {
        do {
 
     if (nfds && (fds->revents & POLLIN)) {
        do {
-           FILLZERO(from);
            fromlen=sizeof(from);
            BUF_ASSERT_FREE(st->rbuf);
            BUF_ALLOC(st->rbuf,"udp_afterpoll");
            fromlen=sizeof(from);
            BUF_ASSERT_FREE(st->rbuf);
            BUF_ALLOC(st->rbuf,"udp_afterpoll");
@@ -110,7 +109,6 @@ static void udp_afterpoll(void *state, struct pollfd *fds, int nfds)
                    memcpy(&from.sin.sin_port,buf_unprepend(st->rbuf,2),2);
                }
                struct comm_addr ca;
                    memcpy(&from.sin.sin_port,buf_unprepend(st->rbuf,2),2);
                }
                struct comm_addr ca;
-               FILLZERO(ca);
                ca.comm=&st->ops;
                ca.ia=from;
                done=False;
                ca.comm=&st->ops;
                ca.ia=from;
                done=False;
@@ -288,7 +286,6 @@ static list_t *udp_apply(closure_t *self, struct cloc loc, dict_t *context,
     st->ops.release_notify=release_notify;
     st->ops.sendmsg=udp_sendmsg;
     st->ops.addr_to_string=addr_to_string;
     st->ops.release_notify=release_notify;
     st->ops.sendmsg=udp_sendmsg;
     st->ops.addr_to_string=addr_to_string;
-    FILLZERO(st->addr);
     st->use_proxy=False;
 
     i=list_elem(args,0);
     st->use_proxy=False;
 
     i=list_elem(args,0);
@@ -306,7 +303,6 @@ static list_t *udp_apply(closure_t *self, struct cloc loc, dict_t *context,
     l=dict_lookup(d,"proxy");
     if (l) {
        st->use_proxy=True;
     l=dict_lookup(d,"proxy");
     if (l) {
        st->use_proxy=True;
-       FILLZERO(st->proxy);
        st->proxy.sa.sa_family=AF_INET;
        i=list_elem(l,0);
        if (!i || i->type!=t_string) {
        st->proxy.sa.sa_family=AF_INET;
        i=list_elem(l,0);
        if (!i || i->type!=t_string) {
diff --git a/util.h b/util.h
index c4e34a8..e321814 100644 (file)
--- a/util.h
+++ b/util.h
@@ -82,7 +82,6 @@ extern void send_nak(const struct comm_addr *dest, uint32_t our_index,
 extern int consttime_memeq(const void *s1, const void *s2, size_t n);
 
 const char *iaddr_to_string(const union iaddr *ia);
 extern int consttime_memeq(const void *s1, const void *s2, size_t n);
 
 const char *iaddr_to_string(const union iaddr *ia);
-bool_t iaddr_equal(const union iaddr *ia, const union iaddr *ib);
 int iaddr_socklen(const union iaddr *ia);
 
 #define MINMAX(ae,be,op) ({                    \
 int iaddr_socklen(const union iaddr *ia);
 
 #define MINMAX(ae,be,op) ({                    \