[PATCH v4 4/9] make conf_ports() exit immediately after logging error

[Laine Stump <laine@redhat.com>]

Rather than having conf_ports() (possibly) log an error, and then
letting the caller log the entire usage() message and exit, just log
the errors and exit immediately (using die()).

For some errors, conf_ports would previously not log any specific
message, leaving it up to the user to determine the problem by
guessing. We replace all of those silent returns with die()
(logging a specific error), thus permitting us to make conf_ports()
return void, which simplifies the caller.

While modifying the two callers to conf_ports() to not check for a
return value, we can further simplify the code by removing the check
for a non-null optarg, as that is guaranteed to never happen (due to
prior calls to getopt_long() with "argument required" for all relevant
options - getopt_long() would have already caught this error).

Signed-off-by: Laine Stump <laine@redhat.com>
---
 conf.c | 52 ++++++++++++++++++++++++++++------------------------
 1 file changed, 28 insertions(+), 24 deletions(-)

diff --git a/conf.c b/conf.c
index ad8c249..0d4ff79 100644
--- a/conf.c
+++ b/conf.c
@@ -173,11 +173,9 @@ static int parse_port_range(const char *s, char **endptr,
  * @optname:	Short option name, t, T, u, or U
  * @optarg:	Option argument (port specification)
  * @fwd:	Pointer to @port_fwd to be updated
- *
- * Return: -EINVAL on parsing error, 0 otherwise
  */
-static int conf_ports(const struct ctx *c, char optname, const char *optarg,
-		      struct port_fwd *fwd)
+static void conf_ports(const struct ctx *c, char optname, const char *optarg,
+		       struct port_fwd *fwd)
 {
 	char addr_buf[sizeof(struct in6_addr)] = { 0 }, *addr = addr_buf;
 	char buf[BUFSIZ], *spec, *ifname = NULL, *p;
@@ -187,23 +185,32 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
 
 	if (!strcmp(optarg, "none")) {
 		if (fwd->mode)
-			return -EINVAL;
+			goto mode_conflict;
+
 		fwd->mode = FWD_NONE;
-		return 0;
+		return;
 	}
 
 	if (!strcmp(optarg, "auto")) {
-		if (fwd->mode || c->mode != MODE_PASTA)
-			return -EINVAL;
+		if (fwd->mode)
+			goto mode_conflict;
+
+		if (c->mode != MODE_PASTA)
+			die("'auto' port forwarding is only allowed for pasta");
+
 		fwd->mode = FWD_AUTO;
-		return 0;
+		return;
 	}
 
 	if (!strcmp(optarg, "all")) {
 		unsigned i;
 
-		if (fwd->mode || c->mode != MODE_PASST)
-			return -EINVAL;
+		if (fwd->mode)
+			goto mode_conflict;
+
+		if (c->mode != MODE_PASST)
+			die("'all' port forwarding is only allowed for passt");
+
 		fwd->mode = FWD_ALL;
 		memset(fwd->map, 0xff, PORT_EPHEMERAL_MIN / 8);
 
@@ -214,11 +221,11 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
 				udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL, i);
 		}
 
-		return 0;
+		return;
 	}
 
 	if (fwd->mode > FWD_SPEC)
-		return -EINVAL;
+		die("Specific ports cannot be specified together with all/none/auto");
 
 	fwd->mode = FWD_SPEC;
 
@@ -292,7 +299,7 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
 				udp_sock_init(c, 0, af, addr, ifname, i);
 		}
 
-		return 0;
+		return;
 	}
 
 	/* Now process base ranges, skipping exclusions */
@@ -339,14 +346,13 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
 		}
 	} while ((p = next_chunk(p, ',')));
 
-	return 0;
+	return;
 bad:
-	err("Invalid port specifier %s", optarg);
-	return -EINVAL;
-
+	die("Invalid port specifier %s", optarg);
 overlap:
-	err("Overlapping port specifier %s", optarg);
-	return -EINVAL;
+	die("Overlapping port specifier %s", optarg);
+mode_conflict:
+	die("Port forwarding mode '%s' conflicts with previous mode", optarg);
 }
 
 /**
@@ -1550,8 +1556,7 @@ void conf(struct ctx *c, int argc, char **argv)
 
 		if ((name == 't' && (fwd = &c->tcp.fwd_in)) ||
 		    (name == 'u' && (fwd = &c->udp.fwd_in.f))) {
-			if (!optarg || conf_ports(c, name, optarg, fwd))
-				usage(argv[0]);
+			conf_ports(c, name, optarg, fwd);
 		}
 	} while (name != -1);
 
@@ -1589,8 +1594,7 @@ void conf(struct ctx *c, int argc, char **argv)
 
 		if ((name == 'T' && (fwd = &c->tcp.fwd_out)) ||
 		    (name == 'U' && (fwd = &c->udp.fwd_out.f))) {
-			if (!optarg || conf_ports(c, name, optarg, fwd))
-				usage(argv[0]);
+			conf_ports(c, name, optarg, fwd);
 		}
 	} while (name != -1);
 
-- 
@@ -173,11 +173,9 @@ static int parse_port_range(const char *s, char **endptr,
  * @optname:	Short option name, t, T, u, or U
  * @optarg:	Option argument (port specification)
  * @fwd:	Pointer to @port_fwd to be updated
- *
- * Return: -EINVAL on parsing error, 0 otherwise
  */
-static int conf_ports(const struct ctx *c, char optname, const char *optarg,
-		      struct port_fwd *fwd)
+static void conf_ports(const struct ctx *c, char optname, const char *optarg,
+		       struct port_fwd *fwd)
 {
 	char addr_buf[sizeof(struct in6_addr)] = { 0 }, *addr = addr_buf;
 	char buf[BUFSIZ], *spec, *ifname = NULL, *p;
@@ -187,23 +185,32 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
 
 	if (!strcmp(optarg, "none")) {
 		if (fwd->mode)
-			return -EINVAL;
+			goto mode_conflict;
+
 		fwd->mode = FWD_NONE;
-		return 0;
+		return;
 	}
 
 	if (!strcmp(optarg, "auto")) {
-		if (fwd->mode || c->mode != MODE_PASTA)
-			return -EINVAL;
+		if (fwd->mode)
+			goto mode_conflict;
+
+		if (c->mode != MODE_PASTA)
+			die("'auto' port forwarding is only allowed for pasta");
+
 		fwd->mode = FWD_AUTO;
-		return 0;
+		return;
 	}
 
 	if (!strcmp(optarg, "all")) {
 		unsigned i;
 
-		if (fwd->mode || c->mode != MODE_PASST)
-			return -EINVAL;
+		if (fwd->mode)
+			goto mode_conflict;
+
+		if (c->mode != MODE_PASST)
+			die("'all' port forwarding is only allowed for passt");
+
 		fwd->mode = FWD_ALL;
 		memset(fwd->map, 0xff, PORT_EPHEMERAL_MIN / 8);
 
@@ -214,11 +221,11 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
 				udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL, i);
 		}
 
-		return 0;
+		return;
 	}
 
 	if (fwd->mode > FWD_SPEC)
-		return -EINVAL;
+		die("Specific ports cannot be specified together with all/none/auto");
 
 	fwd->mode = FWD_SPEC;
 
@@ -292,7 +299,7 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
 				udp_sock_init(c, 0, af, addr, ifname, i);
 		}
 
-		return 0;
+		return;
 	}
 
 	/* Now process base ranges, skipping exclusions */
@@ -339,14 +346,13 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
 		}
 	} while ((p = next_chunk(p, ',')));
 
-	return 0;
+	return;
 bad:
-	err("Invalid port specifier %s", optarg);
-	return -EINVAL;
-
+	die("Invalid port specifier %s", optarg);
 overlap:
-	err("Overlapping port specifier %s", optarg);
-	return -EINVAL;
+	die("Overlapping port specifier %s", optarg);
+mode_conflict:
+	die("Port forwarding mode '%s' conflicts with previous mode", optarg);
 }
 
 /**
@@ -1550,8 +1556,7 @@ void conf(struct ctx *c, int argc, char **argv)
 
 		if ((name == 't' && (fwd = &c->tcp.fwd_in)) ||
 		    (name == 'u' && (fwd = &c->udp.fwd_in.f))) {
-			if (!optarg || conf_ports(c, name, optarg, fwd))
-				usage(argv[0]);
+			conf_ports(c, name, optarg, fwd);
 		}
 	} while (name != -1);
 
@@ -1589,8 +1594,7 @@ void conf(struct ctx *c, int argc, char **argv)
 
 		if ((name == 'T' && (fwd = &c->tcp.fwd_out)) ||
 		    (name == 'U' && (fwd = &c->udp.fwd_out.f))) {
-			if (!optarg || conf_ports(c, name, optarg, fwd))
-				usage(argv[0]);
+			conf_ports(c, name, optarg, fwd);
 		}
 	} while (name != -1);
 
-- 
2.39.1