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
---
conf.c | 203 ++++++++++++++++++++++++++++++++-------------------------
1 file changed, 116 insertions(+), 87 deletions(-)
diff --git a/conf.c b/conf.c
index ae37bf96..c515480b 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;
}
@@ -213,6 +215,101 @@ enum fwd_mode {
FWD_MODE_ALL,
};
+/**
+ * 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
@@ -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);
}
--
2.53.0