Until now, the default_port parameters to these functions have had different types and different behavior. There is a reason for this, since it makes sense to listen on a kernel-selected port but it does not make sense to connect to a kernel-selected port, but this overlooks the possibility that a caller might want to parse a string in the format understood by inet_parse_active() without actually using it to connect to a remote host. This commit makes the behavior consistent and updates all the callers to work with the new semantics.
Signed-off-by: Ben Pfaff <b...@ovn.org> --- lib/socket-util.c | 14 +++++++------- lib/socket-util.h | 4 ++-- lib/stream-tcp.c | 2 +- lib/stream.c | 3 +-- lib/stream.h | 2 +- lib/vlog.c | 2 +- ofproto/collectors.c | 4 ++-- ofproto/collectors.h | 2 +- ofproto/netflow.c | 2 +- ovn/northd/ovn-northd.c | 2 +- ovsdb/raft-private.c | 2 +- 11 files changed, 19 insertions(+), 20 deletions(-) diff --git a/lib/socket-util.c b/lib/socket-util.c index 2d893bc9feb6..223e3780ba5f 100644 --- a/lib/socket-util.c +++ b/lib/socket-util.c @@ -431,13 +431,13 @@ exit: /* Parses 'target', which should be a string in the format "<host>[:<port>]". * <host>, which is required, may be an IPv4 address or an IPv6 address - * enclosed in square brackets. If 'default_port' is nonzero then <port> is - * optional and defaults to 'default_port'. + * enclosed in square brackets. If 'default_port' is nonnegative then <port> + * is optional and defaults to 'default_port'. * * On success, returns true and stores the parsed remote address into '*ss'. * On failure, logs an error, stores zeros into '*ss', and returns false. */ bool -inet_parse_active(const char *target_, uint16_t default_port, +inet_parse_active(const char *target_, int default_port, struct sockaddr_storage *ss) { char *target = xstrdup(target_); @@ -452,7 +452,7 @@ inet_parse_active(const char *target_, uint16_t default_port, if (!host) { VLOG_ERR("%s: host must be specified", target_); ok = false; - } else if (!port && !default_port) { + } else if (!port && default_port < 0) { VLOG_ERR("%s: port must be specified", target_); ok = false; } else if (p && p[strspn(p, " \t\r\n")] != '\0') { @@ -472,8 +472,8 @@ inet_parse_active(const char *target_, uint16_t default_port, /* Opens a non-blocking IPv4 or IPv6 socket of the specified 'style' and * connects to 'target', which should be a string in the format * "<host>[:<port>]". <host>, which is required, may be an IPv4 address or an - * IPv6 address enclosed in square brackets. If 'default_port' is nonzero then - * <port> is optional and defaults to 'default_port'. + * IPv6 address enclosed in square brackets. If 'default_port' is nonnegative + * then <port> is optional and defaults to 'default_port'. * * 'style' should be SOCK_STREAM (for TCP) or SOCK_DGRAM (for UDP). * @@ -488,7 +488,7 @@ inet_parse_active(const char *target_, uint16_t default_port, * should be in the range [0, 63] and will automatically be shifted to the * appropriately place in the IP tos field. */ int -inet_open_active(int style, const char *target, uint16_t default_port, +inet_open_active(int style, const char *target, int default_port, struct sockaddr_storage *ssp, int *fdp, uint8_t dscp) { struct sockaddr_storage ss; diff --git a/lib/socket-util.h b/lib/socket-util.h index b1eca88eb131..d927d67a0e1b 100644 --- a/lib/socket-util.h +++ b/lib/socket-util.h @@ -47,9 +47,9 @@ void drain_fd(int fd, size_t n_packets); ovs_be32 guess_netmask(ovs_be32 ip); char *inet_parse_token(char **); -bool inet_parse_active(const char *target, uint16_t default_port, +bool inet_parse_active(const char *target, int default_port, struct sockaddr_storage *ssp); -int inet_open_active(int style, const char *target, uint16_t default_port, +int inet_open_active(int style, const char *target, int default_port, struct sockaddr_storage *ssp, int *fdp, uint8_t dscp); bool inet_parse_passive(const char *target, int default_port, diff --git a/lib/stream-tcp.c b/lib/stream-tcp.c index 4fae731d9b9e..e8dc2bfaa2df 100644 --- a/lib/stream-tcp.c +++ b/lib/stream-tcp.c @@ -53,7 +53,7 @@ tcp_open(const char *name, char *suffix, struct stream **streamp, uint8_t dscp) { int fd, error; - error = inet_open_active(SOCK_STREAM, suffix, 0, NULL, &fd, dscp); + error = inet_open_active(SOCK_STREAM, suffix, -1, NULL, &fd, dscp); if (fd >= 0) { return new_tcp_stream(xstrdup(name), fd, error, streamp); } else { diff --git a/lib/stream.c b/lib/stream.c index 083e2fb93f77..63c592344080 100644 --- a/lib/stream.c +++ b/lib/stream.c @@ -747,8 +747,7 @@ pstream_open_with_default_port(const char *name_, * - On error, function returns false and *ss contains garbage. */ bool -stream_parse_target_with_default_port(const char *target, - uint16_t default_port, +stream_parse_target_with_default_port(const char *target, int default_port, struct sockaddr_storage *ss) { return ((!strncmp(target, "tcp:", 4) || !strncmp(target, "ssl:", 4)) diff --git a/lib/stream.h b/lib/stream.h index 7d35fa6461ee..88f576155108 100644 --- a/lib/stream.h +++ b/lib/stream.h @@ -78,7 +78,7 @@ int pstream_open_with_default_port(const char *name, struct pstream **, uint8_t dscp); bool stream_parse_target_with_default_port(const char *target, - uint16_t default_port, + int default_port, struct sockaddr_storage *ss); int stream_or_pstream_needs_probes(const char *name); diff --git a/lib/vlog.c b/lib/vlog.c index 0e862a773cf1..bf5fd88ba9e8 100644 --- a/lib/vlog.c +++ b/lib/vlog.c @@ -599,7 +599,7 @@ vlog_set_syslog_target(const char *target) { int new_fd; - inet_open_active(SOCK_DGRAM, target, 0, NULL, &new_fd, 0); + inet_open_active(SOCK_DGRAM, target, -1, NULL, &new_fd, 0); ovs_rwlock_wrlock(&pattern_rwlock); if (syslog_fd >= 0) { diff --git a/ofproto/collectors.c b/ofproto/collectors.c index bc92332b1b4a..bb6ae021fa64 100644 --- a/ofproto/collectors.c +++ b/ofproto/collectors.c @@ -40,7 +40,7 @@ struct collectors { * otherwise a positive errno value if opening at least one collector failed. * * Each target in 'targets' should be a string in the format "<host>[:<port>]". - * <port> may be omitted if 'default_port' is nonzero, in which case it + * <port> may be omitted if 'default_port' is nonnegative, in which case it * defaults to 'default_port'. * * '*collectorsp' is set to a null pointer if no targets were successfully @@ -49,7 +49,7 @@ struct collectors { * is nonnull, and even on a successful return, it is possible that * '*collectorsp' is null, if 'target's is an empty sset. */ int -collectors_create(const struct sset *targets, uint16_t default_port, +collectors_create(const struct sset *targets, int default_port, struct collectors **collectorsp) { struct collectors *c; diff --git a/ofproto/collectors.h b/ofproto/collectors.h index 1e4e96135ff6..83cd26b7ce25 100644 --- a/ofproto/collectors.h +++ b/ofproto/collectors.h @@ -23,7 +23,7 @@ struct collectors; struct sset; -int collectors_create(const struct sset *targets, uint16_t default_port, +int collectors_create(const struct sset *targets, int default_port, struct collectors **); void collectors_destroy(struct collectors *); diff --git a/ofproto/netflow.c b/ofproto/netflow.c index b6baeb8268ab..ed58de17de5e 100644 --- a/ofproto/netflow.c +++ b/ofproto/netflow.c @@ -362,7 +362,7 @@ netflow_set_options(struct netflow *nf, nf->add_id_to_iface = nf_options->add_id_to_iface; collectors_destroy(nf->collectors); - collectors_create(&nf_options->collectors, 0, &nf->collectors); + collectors_create(&nf_options->collectors, -1, &nf->collectors); old_timeout = nf->active_timeout; if (nf_options->active_timeout >= 0) { diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index b518953f2e63..e06d28d6f8fc 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -2942,7 +2942,7 @@ ip_address_and_port_from_lb_key(const char *key, char **ip_address, uint16_t *port, int *addr_family) { struct sockaddr_storage ss; - if (!inet_parse_active(key, 0, &ss)) { + if (!inet_parse_active(key, -1, &ss)) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); VLOG_WARN_RL(&rl, "bad ip address or port for load balancer key %s", key); diff --git a/ovsdb/raft-private.c b/ovsdb/raft-private.c index 36725f7e12b7..07996e35bc6a 100644 --- a/ovsdb/raft-private.c +++ b/ovsdb/raft-private.c @@ -33,7 +33,7 @@ raft_address_validate(const char *address) return NULL; } else if (!strncmp(address, "ssl:", 4) || !strncmp(address, "tcp:", 4)) { struct sockaddr_storage ss; - if (!inet_parse_active(address + 4, 0, &ss)) { + if (!inet_parse_active(address + 4, -1, &ss)) { return ovsdb_error(NULL, "%s: syntax error in address", address); } return NULL; -- 2.16.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev