conf: Split parsing of port specifiers from the rest of -[tuTU] parsing

conf_ports() is extremely long, but we want to refactor it so that parts
can be shared with the upcoming configuration client.  Make a small start
by separating out the section that parses just the port specification
(not including address and/or interface).

This also allows us to constify a few extra things, and while we're there
replace a few vague error messages with more specific ones.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

1 files changed, 116 insertions, 87 deletions

diff --git a/conf.c b/conf.c
index 857c321..e385afc 100644
--- a/conf.c
+++ b/conf.c
@@ -74,7 +74,7 @@ const char *pasta_default_ifn = "tap0";
  *	   character *after* the delimiter, if no further @c is in @s,
  *	   return NULL
  */
-static char *next_chunk(const char *s, char c)
+static const char *next_chunk(const char *s, char c)
 {
 	char *sep = strchr(s, c);
 	return sep ? sep + 1 : NULL;
@@ -101,18 +101,19 @@ struct port_range {
  * Return: -EINVAL on parsing error, -ERANGE on out of range port
  *	   numbers, 0 on success
  */
-static int parse_port_range(const char *s, char **endptr,
+static int parse_port_range(const char *s, const char **endptr,
 			    struct port_range *range)
 {
 	unsigned long first, last;
+	char *ep;
 
-	last = first = strtoul(s, endptr, 10);
-	if (*endptr == s) /* Parsed nothing */
+	last = first = strtoul(s, &ep, 10);
+	if (ep == s) /* Parsed nothing */
 		return -EINVAL;
-	if (**endptr == '-') { /* we have a last value too */
-		const char *lasts = *endptr + 1;
-		last = strtoul(lasts, endptr, 10);
-		if (*endptr == lasts) /* Parsed nothing */
+	if (*ep == '-') { /* we have a last value too */
+		const char *lasts = ep + 1;
+		last = strtoul(lasts, &ep, 10);
+		if (ep == lasts) /* Parsed nothing */
 			return -EINVAL;
 	}
 
@@ -121,6 +122,7 @@ static int parse_port_range(const char *s, char **endptr,
 
 	range->first = first;
 	range->last = last;
+	*endptr = ep;
 
 	return 0;
 }
@@ -214,6 +216,101 @@ enum fwd_mode {
 };
 
 /**
+ * conf_ports_spec() - Parse port range(s) specifier
+ * @c:		Execution context
+ * @optname:	Short option name, t, T, u, or U
+ * @optarg:	Option argument (port specification)
+ * @fwd:	Forwarding table to be updated
+ * @addr:	Listening address for forwarding
+ * @ifname:	Interface name for listening
+ * @spec:	Port range(s) specifier
+ */
+static void conf_ports_spec(const struct ctx *c,
+			    char optname, const char *optarg,
+			    struct fwd_table *fwd,
+			    const union inany_addr *addr, const char *ifname,
+			    const char *spec)
+{
+	uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
+	bool exclude_only = true;
+	const char *p;
+	unsigned i;
+
+	/* Mark all exclusions first, they might be given after base ranges */
+	p = spec;
+	do {
+		struct port_range xrange;
+
+		if (*p != '~') {
+			/* Not an exclude range, parse later */
+			exclude_only = false;
+			continue;
+		}
+		p++;
+
+		if (parse_port_range(p, &p, &xrange))
+			goto bad;
+		if ((*p != '\0')  && (*p != ',')) /* Garbage after the range */
+			goto bad;
+
+		for (i = xrange.first; i <= xrange.last; i++)
+			bitmap_set(exclude, i);
+	} while ((p = next_chunk(p, ',')));
+
+	if (exclude_only) {
+		/* Exclude ephemeral ports */
+		fwd_port_map_ephemeral(exclude);
+
+		conf_ports_range_except(c, optname, optarg, fwd,
+					addr, ifname,
+					1, NUM_PORTS - 1, exclude,
+					1, FWD_WEAK);
+		return;
+	}
+
+	/* Now process base ranges, skipping exclusions */
+	p = spec;
+	do {
+		struct port_range orig_range, mapped_range;
+
+		if (*p == '~')
+			/* Exclude range, already parsed */
+			continue;
+
+		if (parse_port_range(p, &p, &orig_range))
+			goto bad;
+
+		if (*p == ':') { /* There's a range to map to as well */
+			if (parse_port_range(p + 1, &p, &mapped_range))
+				goto bad;
+			if ((mapped_range.last - mapped_range.first) !=
+			    (orig_range.last - orig_range.first))
+				goto bad;
+		} else {
+			mapped_range = orig_range;
+		}
+
+		if ((*p != '\0')  && (*p != ',')) /* Garbage after the ranges */
+			goto bad;
+
+		if (orig_range.first == 0) {
+			die("Can't forward port 0 for option '-%c %s'",
+			    optname, optarg);
+		}
+
+		conf_ports_range_except(c, optname, optarg, fwd,
+					addr, ifname,
+					orig_range.first, orig_range.last,
+					exclude,
+					mapped_range.first, 0);
+	} while ((p = next_chunk(p, ',')));
+
+	return;
+bad:
+	die("Invalid port specifier %s", optarg);
+}
+
+/**
  * conf_ports() - Parse port configuration options, initialise UDP/TCP sockets
  * @c:		Execution context
  * @optname:	Short option name, t, T, u, or U
@@ -226,9 +323,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 {
 	union inany_addr addr_buf = inany_any6, *addr = &addr_buf;
 	char buf[BUFSIZ], *spec, *ifname = NULL, *p;
-	uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
-	bool exclude_only = true;
-	unsigned i;
 
 	if (!strcmp(optarg, "none")) {
 		if (*mode)
@@ -255,6 +349,8 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 	}
 
 	if (!strcmp(optarg, "all")) {
+		uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
+
 		if (*mode)
 			goto mode_conflict;
 
@@ -285,7 +381,8 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		spec++;
 
 		if (optname != 't' && optname != 'u')
-			goto bad;
+			die("Listening address not allowed for -%c %s",
+			    optname, optarg);
 
 		if ((ifname = strchr(buf, '%'))) {
 			*ifname = 0;
@@ -295,9 +392,10 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 			 * so the length of the given ifname is:
 			 * (spec - ifname - 1)
 			 */
-			if (spec - ifname - 1 >= IFNAMSIZ)
-				goto bad;
-
+			if (spec - ifname - 1 >= IFNAMSIZ) {
+				die("Interface name '%s' is too long (max %u)",
+				    ifname, IFNAMSIZ - 1);
+			}
 		}
 
 		if (ifname == buf + 1) {	/* Interface without address */
@@ -312,7 +410,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 			}
 
 			if (!inany_pton(p, addr))
-				goto bad;
+				die("Bad forwarding address '%s'", p);
 		}
 	} else {
 		spec = buf;
@@ -330,27 +428,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		}
 	}
 
-	/* Mark all exclusions first, they might be given after base ranges */
-	p = spec;
-	do {
-		struct port_range xrange;
-
-		if (*p != '~') {
-			/* Not an exclude range, parse later */
-			exclude_only = false;
-			continue;
-		}
-		p++;
-
-		if (parse_port_range(p, &p, &xrange))
-			goto bad;
-		if ((*p != '\0')  && (*p != ',')) /* Garbage after the range */
-			goto bad;
-
-		for (i = xrange.first; i <= xrange.last; i++)
-			bitmap_set(exclude, i);
-	} while ((p = next_chunk(p, ',')));
-
 	if (ifname && c->no_bindtodevice) {
 		die(
 "Device binding for '-%c %s' unsupported (requires kernel 5.7+)",
@@ -360,57 +437,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 	if ((optname == 'T' || optname == 'U') && !ifname)
 		ifname = "lo";
 
-	if (exclude_only) {
-		/* Exclude ephemeral ports */
-		fwd_port_map_ephemeral(exclude);
-
-		conf_ports_range_except(c, optname, optarg, fwd,
-					addr, ifname,
-					1, NUM_PORTS - 1, exclude,
-					1, FWD_WEAK);
-		return;
-	}
-
-	/* Now process base ranges, skipping exclusions */
-	p = spec;
-	do {
-		struct port_range orig_range, mapped_range;
-
-		if (*p == '~')
-			/* Exclude range, already parsed */
-			continue;
-
-		if (parse_port_range(p, &p, &orig_range))
-			goto bad;
-
-		if (*p == ':') { /* There's a range to map to as well */
-			if (parse_port_range(p + 1, &p, &mapped_range))
-				goto bad;
-			if ((mapped_range.last - mapped_range.first) !=
-			    (orig_range.last - orig_range.first))
-				goto bad;
-		} else {
-			mapped_range = orig_range;
-		}
-
-		if ((*p != '\0')  && (*p != ',')) /* Garbage after the ranges */
-			goto bad;
-
-		if (orig_range.first == 0) {
-			die("Can't forward port 0 for option '-%c %s'",
-			    optname, optarg);
-		}
-
-		conf_ports_range_except(c, optname, optarg, fwd,
-					addr, ifname,
-					orig_range.first, orig_range.last,
-					exclude,
-					mapped_range.first, 0);
-	} while ((p = next_chunk(p, ',')));
-
+	conf_ports_spec(c, optname, optarg, fwd, addr, ifname, spec);
 	return;
-bad:
-	die("Invalid port specifier %s", optarg);
+
 mode_conflict:
 	die("Port forwarding mode '%s' conflicts with previous mode", optarg);
 }