packets: Change IP_ARGS interface to take an ovs_be32 instead of a pointer.

An ovs_be32 is a more obvious way to represent an IP address than a
pointer to one.  It is also more type-safe, especially since "sparse" is
able to check that the argument is in network byte order.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>

13 files changed, 30 insertions, 34 deletions

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 9b59bc9a..d9a444b5 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -2338,7 +2338,7 @@ netdev_linux_arp_lookup(const struct netdev *netdev,
         memcpy(mac, r.arp_ha.sa_data, ETH_ADDR_LEN);
     } else if (retval != ENXIO) {
         VLOG_WARN_RL(&rl, "%s: could not look up ARP entry for "IP_FMT": %s",
-                     netdev_get_name(netdev), IP_ARGS(&ip), strerror(retval));
+                     netdev_get_name(netdev), IP_ARGS(ip), strerror(retval));
     }
     return retval;
 }
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 530fd206..2b493a7f 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -810,11 +810,11 @@ unparse_tunnel_config(const char *name OVS_UNUSED, const char *type OVS_UNUSED,
 
 
     daddr = nl_attr_get_be32(a[OVS_TUNNEL_ATTR_DST_IPV4]);
-    smap_add_format(args, "remote_ip", IP_FMT, IP_ARGS(&daddr));
+    smap_add_format(args, "remote_ip", IP_FMT, IP_ARGS(daddr));
 
     if (a[OVS_TUNNEL_ATTR_SRC_IPV4]) {
         ovs_be32 saddr = nl_attr_get_be32(a[OVS_TUNNEL_ATTR_SRC_IPV4]);
-        smap_add_format(args, "local_ip", IP_FMT, IP_ARGS(&saddr));
+        smap_add_format(args, "local_ip", IP_FMT, IP_ARGS(saddr));
     }
 
     if (!a[OVS_TUNNEL_ATTR_IN_KEY] && !a[OVS_TUNNEL_ATTR_OUT_KEY]) {
diff --git a/lib/odp-util.c b/lib/odp-util.c
index de97fd21..f1075e35 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -739,8 +739,8 @@ format_odp_key_attr(const struct nlattr *a, struct ds *ds)
         ds_put_format(ds, "(tun_id=0x%"PRIx64",src="IP_FMT",dst="IP_FMT","
                       "tos=0x%"PRIx8",ttl=%"PRIu8",flags(",
                       ntohll(ipv4_tun_key->tun_id),
-                      IP_ARGS(&ipv4_tun_key->ipv4_src),
-                      IP_ARGS(&ipv4_tun_key->ipv4_dst),
+                      IP_ARGS(ipv4_tun_key->ipv4_src),
+                      IP_ARGS(ipv4_tun_key->ipv4_dst),
                       ipv4_tun_key->ipv4_tos, ipv4_tun_key->ipv4_ttl);
 
         format_flags(ds, odp_tun_flag_to_string,
@@ -774,8 +774,8 @@ format_odp_key_attr(const struct nlattr *a, struct ds *ds)
         ipv4_key = nl_attr_get(a);
         ds_put_format(ds, "(src="IP_FMT",dst="IP_FMT",proto=%"PRIu8
                       ",tos=%#"PRIx8",ttl=%"PRIu8",frag=%s)",
-                      IP_ARGS(&ipv4_key->ipv4_src),
-                      IP_ARGS(&ipv4_key->ipv4_dst),
+                      IP_ARGS(ipv4_key->ipv4_src),
+                      IP_ARGS(ipv4_key->ipv4_dst),
                       ipv4_key->ipv4_proto, ipv4_key->ipv4_tos,
                       ipv4_key->ipv4_ttl,
                       ovs_frag_type_to_string(ipv4_key->ipv4_frag));
@@ -826,7 +826,7 @@ format_odp_key_attr(const struct nlattr *a, struct ds *ds)
         arp_key = nl_attr_get(a);
         ds_put_format(ds, "(sip="IP_FMT",tip="IP_FMT",op=%"PRIu16","
                       "sha="ETH_ADDR_FMT",tha="ETH_ADDR_FMT")",
-                      IP_ARGS(&arp_key->arp_sip), IP_ARGS(&arp_key->arp_tip),
+                      IP_ARGS(arp_key->arp_sip), IP_ARGS(arp_key->arp_tip),
                       ntohs(arp_key->arp_op), ETH_ADDR_ARGS(arp_key->arp_sha),
                       ETH_ADDR_ARGS(arp_key->arp_tha));
         break;
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index b758c7c9..186275cd 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -1940,12 +1940,12 @@ ofpact_format(const struct ofpact *a, struct ds *s)
 
     case OFPACT_SET_IPV4_SRC:
         ds_put_format(s, "mod_nw_src:"IP_FMT,
-                      IP_ARGS(&ofpact_get_SET_IPV4_SRC(a)->ipv4));
+                      IP_ARGS(ofpact_get_SET_IPV4_SRC(a)->ipv4));
         break;
 
     case OFPACT_SET_IPV4_DST:
         ds_put_format(s, "mod_nw_dst:"IP_FMT,
-                      IP_ARGS(&ofpact_get_SET_IPV4_DST(a)->ipv4));
+                      IP_ARGS(ofpact_get_SET_IPV4_DST(a)->ipv4));
         break;
 
     case OFPACT_SET_IPV4_DSCP:
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 84c37cff..f45ffa4d 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -578,7 +578,7 @@ print_ip_netmask(struct ds *string, const char *leader, ovs_be32 ip,
     }
     ds_put_cstr(string, leader);
     if (wild_bits < 32) {
-        ds_put_format(string, IP_FMT, IP_ARGS(&ip));
+        ds_put_format(string, IP_FMT, IP_ARGS(ip));
         if (wild_bits) {
             ds_put_format(string, "/%d", 32 - wild_bits);
         }
diff --git a/lib/packets.c b/lib/packets.c
index fa73b50a..812d1af9 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -273,12 +273,12 @@ ip_count_cidr_bits(ovs_be32 netmask)
 void
 ip_format_masked(ovs_be32 ip, ovs_be32 mask, struct ds *s)
 {
-    ds_put_format(s, IP_FMT, IP_ARGS(&ip));
+    ds_put_format(s, IP_FMT, IP_ARGS(ip));
     if (mask != htonl(UINT32_MAX)) {
         if (ip_is_cidr(mask)) {
             ds_put_format(s, "/%d", ip_count_cidr_bits(mask));
         } else {
-            ds_put_format(s, "/"IP_FMT, IP_ARGS(&mask));
+            ds_put_format(s, "/"IP_FMT, IP_ARGS(mask));
         }
     }
 }
diff --git a/lib/packets.h b/lib/packets.h
index 5f9be01e..7e2d4e99 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -272,16 +272,12 @@ struct vlan_eth_header {
 } __attribute__((packed));
 BUILD_ASSERT_DECL(VLAN_ETH_HEADER_LEN == sizeof(struct vlan_eth_header));
 
-/* The "(void) (ip)[0]" below has no effect on the value, since it's the first
- * argument of a comma expression, but it makes sure that 'ip' is a pointer.
- * This is useful since a common mistake is to pass an integer instead of a
- * pointer to IP_ARGS. */
-#define IP_FMT "%"PRIu8".%"PRIu8".%"PRIu8".%"PRIu8
+#define IP_FMT "%"PRIu32".%"PRIu32".%"PRIu32".%"PRIu32
 #define IP_ARGS(ip)                             \
-        ((void) (ip)[0], ((uint8_t *) ip)[0]),  \
-        ((uint8_t *) ip)[1],                    \
-        ((uint8_t *) ip)[2],                    \
-        ((uint8_t *) ip)[3]
+    ntohl(ip) >> 24,                            \
+    (ntohl(ip) >> 16) & 0xff,                   \
+    (ntohl(ip) >> 8) & 0xff,                    \
+    ntohl(ip) & 0xff
 
 /* Example:
  *
diff --git a/lib/socket-util.c b/lib/socket-util.c
index 48f42f19..d31bc769 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -941,7 +941,7 @@ describe_sockaddr(struct ds *string, int fd,
 
             memcpy(&sin, &ss, sizeof sin);
             ds_put_format(string, IP_FMT":%"PRIu16,
-                          IP_ARGS(&sin.sin_addr.s_addr), ntohs(sin.sin_port));
+                          IP_ARGS(sin.sin_addr.s_addr), ntohs(sin.sin_port));
         } else if (ss.ss_family == AF_UNIX) {
             struct sockaddr_un sun;
             const char *null;
diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index 0ca5b18d..184b3ff4 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -805,7 +805,7 @@ pssl_open(const char *name OVS_UNUSED, char *suffix, struct pstream **pstreamp,
         return -fd;
     }
     sprintf(bound_name, "pssl:%"PRIu16":"IP_FMT,
-            ntohs(sin.sin_port), IP_ARGS(&sin.sin_addr.s_addr));
+            ntohs(sin.sin_port), IP_ARGS(sin.sin_addr.s_addr));
 
     pssl = xmalloc(sizeof *pssl);
     pstream_init(&pssl->pstream, &pssl_pstream_class, bound_name);
@@ -847,7 +847,7 @@ pssl_accept(struct pstream *pstream, struct stream **new_streamp)
         return error;
     }
 
-    sprintf(name, "ssl:"IP_FMT, IP_ARGS(&sin.sin_addr));
+    sprintf(name, "ssl:"IP_FMT, IP_ARGS(sin.sin_addr.s_addr));
     if (sin.sin_port != htons(OFP_SSL_PORT)) {
         sprintf(strchr(name, '\0'), ":%"PRIu16, ntohs(sin.sin_port));
     }
diff --git a/lib/stream-tcp.c b/lib/stream-tcp.c
index 05601e19..0384c42f 100644
--- a/lib/stream-tcp.c
+++ b/lib/stream-tcp.c
@@ -116,7 +116,7 @@ ptcp_open(const char *name OVS_UNUSED, char *suffix, struct pstream **pstreamp,
     }
 
     sprintf(bound_name, "ptcp:%"PRIu16":"IP_FMT,
-            ntohs(sin.sin_port), IP_ARGS(&sin.sin_addr.s_addr));
+            ntohs(sin.sin_port), IP_ARGS(sin.sin_addr.s_addr));
     return new_fd_pstream(bound_name, fd, ptcp_accept, set_dscp, NULL,
                           pstreamp);
 }
@@ -129,7 +129,7 @@ ptcp_accept(int fd, const struct sockaddr *sa, size_t sa_len,
     char name[128];
 
     if (sa_len == sizeof(struct sockaddr_in) && sin->sin_family == AF_INET) {
-        sprintf(name, "tcp:"IP_FMT, IP_ARGS(&sin->sin_addr));
+        sprintf(name, "tcp:"IP_FMT, IP_ARGS(sin->sin_addr.s_addr));
         sprintf(strchr(name, '\0'), ":%"PRIu16, ntohs(sin->sin_port));
     } else {
         strcpy(name, "tcp");
diff --git a/ofproto/in-band.c b/ofproto/in-band.c
index 0e4754cf..81b330d2 100644
--- a/ofproto/in-band.c
+++ b/ofproto/in-band.c
@@ -120,7 +120,7 @@ refresh_remote(struct in_band *ib, struct in_band_remote *r)
                                  &next_hop_inaddr, &next_hop_dev);
     if (retval) {
         VLOG_WARN("cannot find route for controller ("IP_FMT"): %s",
-                  IP_ARGS(&r->remote_addr.sin_addr), strerror(retval));
+                  IP_ARGS(r->remote_addr.sin_addr.s_addr), strerror(retval));
         return 1;
     }
     if (!next_hop_inaddr.s_addr) {
@@ -137,7 +137,7 @@ refresh_remote(struct in_band *ib, struct in_band_remote *r)
         if (retval) {
             VLOG_WARN_RL(&rl, "cannot open netdev %s (next hop "
                          "to controller "IP_FMT"): %s",
-                         next_hop_dev, IP_ARGS(&r->remote_addr.sin_addr),
+                         next_hop_dev, IP_ARGS(r->remote_addr.sin_addr.s_addr),
                          strerror(retval));
             free(next_hop_dev);
             return 1;
@@ -150,7 +150,7 @@ refresh_remote(struct in_band *ib, struct in_band_remote *r)
                                r->remote_mac);
     if (retval) {
         VLOG_DBG_RL(&rl, "cannot look up remote MAC address ("IP_FMT"): %s",
-                    IP_ARGS(&next_hop_inaddr.s_addr), strerror(retval));
+                    IP_ARGS(next_hop_inaddr.s_addr), strerror(retval));
     }
 
     /* If we don't have a MAC address, then refresh quickly, since we probably
diff --git a/tests/test-netflow.c b/tests/test-netflow.c
index c37eeafe..5147a20c 100644
--- a/tests/test-netflow.c
+++ b/tests/test-netflow.c
@@ -75,7 +75,7 @@ print_netflow(struct ofpbuf *buf)
         }
 
         printf("seq %"PRIu32": "IP_FMT" > "IP_FMT, ntohl(hdr->flow_seq),
-               IP_ARGS(&rec->src_addr), IP_ARGS(&rec->dst_addr));
+               IP_ARGS(rec->src_addr), IP_ARGS(rec->dst_addr));
 
         printf(", if %"PRIu16" > %"PRIu16,
                ntohs(rec->input), ntohs(rec->output));
@@ -137,7 +137,7 @@ print_netflow(struct ofpbuf *buf)
                ntohl(rec->init_time), ntohl(rec->used_time));
 
         if (rec->nexthop != htonl(0)) {
-            printf(", nexthop "IP_FMT, IP_ARGS(&rec->nexthop));
+            printf(", nexthop "IP_FMT, IP_ARGS(rec->nexthop));
         }
         if (rec->src_as != htons(0) || rec->dst_as != htons(0)) {
             printf(", AS %"PRIu16" > %"PRIu16,
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index f508d58c..d23caf25 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2712,7 +2712,7 @@ bridge_configure_local_iface_netdev(struct bridge *br,
     }
     if (!netdev_set_in4(netdev, ip, mask)) {
         VLOG_INFO("bridge %s: configured IP address "IP_FMT", netmask "IP_FMT,
-                  br->name, IP_ARGS(&ip.s_addr), IP_ARGS(&mask.s_addr));
+                  br->name, IP_ARGS(ip.s_addr), IP_ARGS(mask.s_addr));
     }
 
     /* Configure the default gateway. */
@@ -2721,7 +2721,7 @@ bridge_configure_local_iface_netdev(struct bridge *br,
         && gateway.s_addr) {
         if (!netdev_add_router(netdev, gateway)) {
             VLOG_INFO("bridge %s: configured gateway "IP_FMT,
-                      br->name, IP_ARGS(&gateway.s_addr));
+                      br->name, IP_ARGS(gateway.s_addr));
         }
     }
 }