socket-util: Make inet_parse_active() and inet_parse_passive() more a…

…like.

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 <[email protected]>
Acked-by: Mark Michelson <[email protected]>

lib/socket-util.c

@@ -431,13 +431,13 @@ parse_sockaddr_components(struct sockaddr_storage *ss,
 
 /* 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;

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,

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 {

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))

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);
 

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) {

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;

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 *);
 

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) {

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);

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;