Acked-by: Ethan Jackson <[email protected]>
On Thu, Nov 8, 2012 at 5:12 PM, Ben Pfaff <[email protected]> wrote: > 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 <[email protected]> > --- > lib/netdev-linux.c | 2 +- > lib/netdev-vport.c | 4 ++-- > lib/odp-util.c | 10 +++++----- > lib/ofp-actions.c | 4 ++-- > lib/ofp-print.c | 2 +- > lib/packets.c | 4 ++-- > lib/packets.h | 14 +++++--------- > lib/socket-util.c | 2 +- > lib/stream-ssl.c | 4 ++-- > lib/stream-tcp.c | 4 ++-- > ofproto/in-band.c | 6 +++--- > tests/test-netflow.c | 4 ++-- > vswitchd/bridge.c | 4 ++-- > 13 files changed, 30 insertions(+), 34 deletions(-) > > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > index 0460c06..35b4562 100644 > --- a/lib/netdev-linux.c > +++ b/lib/netdev-linux.c > @@ -2327,7 +2327,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 5171171..da3f733 100644 > --- a/lib/netdev-vport.c > +++ b/lib/netdev-vport.c > @@ -790,11 +790,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 08823e2..978fd9a 100644 > --- a/lib/odp-util.c > +++ b/lib/odp-util.c > @@ -706,8 +706,8 @@ format_odp_key_attr(const struct nlattr *a, struct ds > *ds) > ds_put_format(ds, "(tun_id=0x%"PRIx64",flags=0x%"PRIx32 > > ",src="IP_FMT",dst="IP_FMT",tos=0x%"PRIx8",ttl=%"PRIu8")", > ntohll(ipv4_tun_key->tun_id), > ipv4_tun_key->tun_flags, > - 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); > break; > > @@ -737,8 +737,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)); > @@ -789,7 +789,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 170e796..d192507 100644 > --- a/lib/ofp-actions.c > +++ b/lib/ofp-actions.c > @@ -1930,12 +1930,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 c1b50e2..60f0a41 100644 > --- a/lib/ofp-print.c > +++ b/lib/ofp-print.c > @@ -571,7 +571,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 16f4fe6..8850fa0 100644 > --- a/lib/packets.c > +++ b/lib/packets.c > @@ -272,12 +272,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 e550be0..72d1843 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 4edf956..583bd0a 100644 > --- a/lib/socket-util.c > +++ b/lib/socket-util.c > @@ -945,7 +945,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 0ca5b18..184b3ff 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 05601e1..0384c42 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 0e4754c..81b330d 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 c37eeaf..5147a20 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 8f544a9..0dd16c1 100644 > --- a/vswitchd/bridge.c > +++ b/vswitchd/bridge.c > @@ -2691,7 +2691,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. */ > @@ -2700,7 +2700,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)); > } > } > } > -- > 1.7.2.5 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev >
_______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
