comm etc.: Provide comm_addr_equal

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>

diff --git a/resolver.c b/resolver.c
index b8a7a34289aa13b99e635f829e6e898ebfcb2cb7..d6bc61901085d78d3492bfa411f99c32957d9b56 100644 (file)
--- a/resolver.c
+++ b/resolver.c
@@ -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;
-       FILLZERO(ca);
        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];
-               FILLZERO(ca_buf);
                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;
-                   /* copy fields individually so we leave holes zeroed: */
                    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;
+                       /* 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);

diff --git a/secnet.h b/secnet.h
index f458f8a345724e3cdb78b2dcf25b7ac1b873261f..3fba00f406862d14abe59e97e54dad5cdfe12b58 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. */
-    /* 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;
 };
@@ -367,11 +362,19 @@ struct comm_if {
     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 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

diff --git a/site.c b/site.c
index d3eb662a144d0c6b0b23b821b4fa79554711d50b..d360699214bdd5b54bffa8738193ce06099b9a5d 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;
@@ -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++)
-       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;
@@ -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 &&
-       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;

diff --git a/udp.c b/udp.c
index 42169221fd03a6760909a7e23fb3ff04b6757756..477dea38c7efe47e97ab5f52656fc79c07953509 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 {
-           FILLZERO(from);
            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;
-               FILLZERO(ca);
                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;
-    FILLZERO(st->addr);
     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;
-       FILLZERO(st->proxy);
        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 c4e34a8f2458f619f99e6ff23b709d846a6aeec7..e3218142cbf27a3c98517300185d3d359d9c3749 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);
-bool_t iaddr_equal(const union iaddr *ia, const union iaddr *ib);
 int iaddr_socklen(const union iaddr *ia);
 
 #define MINMAX(ae,be,op) ({                    \