[ovs-dev,05/11] socket-util: Make inet_parse_active() and inet_parse_passive() more alike.

Message ID 20180413172655.31638-5-blp@ovn.org
State Accepted
Headers show
Series
  • [ovs-dev,01/11] socket-util: Fix error in comment on ss_format_address().
Related show

Commit Message

Ben Pfaff April 13, 2018, 5:26 p.m.
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 <blp@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(-)

Patch

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;