From: Stefano Brivio <sbrivio@redhat.com>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: passt-dev@passt.top, Jon Maloy <jamloy@redhat.com>
Subject: Re: [PATCH 08/12] parse: Add helpers for parsing IP addresses
Date: Wed, 01 Jul 2026 02:07:26 +0200 (CEST) [thread overview]
Message-ID: <20260701020725.024edf81@elisabeth> (raw)
In-Reply-To: <20260626071003.3472194-9-david@gibson.dropbear.id.au>
On Fri, 26 Jun 2026 17:09:59 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:
> parse_ipv[46]() are wrappers around inet_pton() that are more
> convenient for use when the IP is part of a longer string, rather than
> the entire string. parse_inany() replaces inany_pton() which again
> will become more convenient for strings including IPs that aren't just
> an IP.
>
> For now we only have some simple and sometimes awkward use cases,
> we'll replace these with more natural uses in future.
>
> Cc: Jon Maloy <jamloy@redhat.com>
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> Makefile | 1 +
> conf.c | 6 ++--
> fwd_rule.c | 21 ++++---------
> inany.c | 24 ++------------
> inany.h | 1 -
> parse.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> parse.h | 4 +++
> 7 files changed, 109 insertions(+), 39 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 85d279c0..e2b22ddf 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -227,4 +227,5 @@ pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:inany.c
> pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:ip.h
> pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:serialise.c
> pesto.cppcheck: CPPCHECK_FLAGS += --suppress=staticFunction:fwd_rule.c
> +pesto.cppcheck: CPPCHECK_FLAGS += --suppress=staticFunction:parse.c
> pesto.cppcheck: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h
> diff --git a/conf.c b/conf.c
> index bd357117..2d1895d4 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -357,7 +357,7 @@ static uint8_t conf_ip4_prefix(const char *arg)
> struct in_addr mask;
> unsigned long len;
>
> - if (inet_pton(AF_INET, arg, &mask)) {
> + if (parse_ipv4(&p, &mask) && parse_eoi(p)) {
> in_addr_t hmask = ntohl(mask.s_addr);
> len = __builtin_popcount(hmask);
> if ((hmask << len) == 0)
> @@ -1577,7 +1577,9 @@ void conf(struct ctx *c, int argc, char **argv)
> if (addr_has_prefix_len && prefix_len_from_opt)
> die("Redundant prefix length specification");
>
> - if (!addr_has_prefix_len && !inany_pton(optarg, &addr))
> + p = optarg;
> + if (!addr_has_prefix_len &&
> + !(parse_inany(&p, &addr) && parse_eoi(p)))
parse_port_range() reports error if the input isn't NULL-terminated,
but these new functions don't, and in all the usages at least
introduced in this patch, you always need to couple that with a &&
parse_eoi(p) (or && !*p).
Is there a reason for this difference?
> die("Invalid address: %s", optarg);
>
> if (prefix_len_from_opt && inany_v4(&addr))
> diff --git a/fwd_rule.c b/fwd_rule.c
> index 0b85b17e..6d7ec2c5 100644
> --- a/fwd_rule.c
> +++ b/fwd_rule.c
> @@ -595,6 +595,8 @@ void fwd_rule_parse(char optname, bool del, const char *optarg,
> strncpy(buf, optarg, sizeof(buf) - 1);
>
> if ((spec = strchr(buf, '/'))) {
> + const char *p = buf;
> +
> *spec = 0;
> spec++;
>
> @@ -616,22 +618,11 @@ void fwd_rule_parse(char optname, bool del, const char *optarg,
> }
> }
>
> - if (ifname == buf + 1) { /* Interface without address */
> + if (ifname == buf + 1 || /* Interface without address */
> + !strcmp(buf, "*")) /* Explicit wildcard address */
> addr = NULL;
> - } else {
> - char *p = buf;
> -
> - /* Allow square brackets for IPv4 too for convenience */
> - if (*p == '[' && p[strlen(p) - 1] == ']') {
> - p[strlen(p) - 1] = '\0';
> - p++;
> - }
> -
> - if (strcmp(p, "*") == 0)
> - addr = NULL;
> - else if (!inany_pton(p, &addr_buf))
> - die("Bad forwarding address '%s'", p);
> - }
> + else if (!parse_inany(&p, &addr_buf) && parse_eoi(p))
> + die("Bad forwarding address '%s'", buf);
> } else {
> spec = buf;
>
> diff --git a/inany.c b/inany.c
> index 23faf3ff..154f08b5 100644
> --- a/inany.c
> +++ b/inany.c
> @@ -27,6 +27,7 @@
> #include "ip.h"
> #include "inany.h"
> #include "fwd.h"
> +#include "parse.h"
>
> const union inany_addr inany_loopback4 = INANY_INIT4(IN4ADDR_LOOPBACK_INIT);
> const union inany_addr inany_any4 = INANY_INIT4(IN4ADDR_ANY_INIT);
> @@ -70,26 +71,6 @@ const char *inany_ntop(const union inany_addr *src, char *dst, socklen_t size)
> return inet_ntop(AF_INET6, &src->a6, dst, size);
> }
>
> -/** inany_pton - Parse an IPv[46] address from text format
> - * @src: IPv[46] address
> - * @dst: output buffer, filled with parsed address
> - *
> - * Return: on success, 1, if no parseable address is found, 0
> - */
> -int inany_pton(const char *src, union inany_addr *dst)
> -{
> - if (inet_pton(AF_INET, src, &dst->v4mapped.a4)) {
> - memset(&dst->v4mapped.zero, 0, sizeof(dst->v4mapped.zero));
> - memset(&dst->v4mapped.one, 0xff, sizeof(dst->v4mapped.one));
> - return 1;
> - }
> -
> - if (inet_pton(AF_INET6, src, &dst->a6))
> - return 1;
> -
> - return 0;
> -}
> -
> /**
> * inany_prefix_pton() - Parse an IPv[46] address with prefix length
> * @src: IPv[46] address and prefix length string in CIDR format
> @@ -104,6 +85,7 @@ int inany_prefix_pton(const char *src, union inany_addr *dst,
> char astr[INANY_ADDRSTRLEN] = { 0 };
> size_t alen = strcspn(src, "/");
> const char *pstr = &src[alen + 1];
> + const char *p = astr;
> unsigned long plen;
> char *end;
>
> @@ -129,7 +111,7 @@ int inany_prefix_pton(const char *src, union inany_addr *dst,
> return 1;
> }
>
> - if (inany_pton(astr, dst)) {
> + if (parse_inany(&p, dst) && parse_eoi(p)) {
> if (plen > 32)
> return 0;
> *prefix_len = plen + 96;
> diff --git a/inany.h b/inany.h
> index 6bf3ecaa..93d98368 100644
> --- a/inany.h
> +++ b/inany.h
> @@ -303,7 +303,6 @@ static inline int inany_from_sockaddr(union inany_addr *dst, in_port_t *port,
>
> bool inany_matches(const union inany_addr *a, const union inany_addr *b);
> const char *inany_ntop(const union inany_addr *src, char *dst, socklen_t size);
> -int inany_pton(const char *src, union inany_addr *dst);
> int inany_prefix_pton(const char *src, union inany_addr *dst,
> uint8_t *prefix_len);
>
> diff --git a/parse.c b/parse.c
> index bd091fde..0349c5dc 100644
> --- a/parse.c
> +++ b/parse.c
> @@ -16,9 +16,12 @@
> #include <stddef.h>
> #include <stdlib.h>
> #include <string.h>
> +#include <limits.h>
> +#include <arpa/inet.h>
>
> #include "common.h"
> #include "parse.h"
> +#include "inany.h"
>
> /**
> * DOC: Theory of Operation
> @@ -110,3 +113,91 @@ bool parse_port_range(const char **cursor, struct port_range *range)
> *cursor = p;
> return true;
> }
> +
> +/**
> + * parse_ipv4() - Parse an IPv4 address from a string
> + * @abuf: On success, updated with parsed address
> + */
> +bool parse_ipv4(const char **cursor, struct in_addr *abuf)
> +{
> + /* Brackets are not typical on IPv4, but allow for consistency */
> + const char *p = *cursor;
> + bool bracket = parse_literal(&p, "[");
> + char buf[INET_ADDRSTRLEN];
> + struct in_addr addr;
> + size_t len;
> +
> + if (bracket)
> + len = strcspn(p, "]");
> + else
> + len = strspn(p, "0123456789.");
> +
> + if (len >= sizeof(buf))
> + return false;
> + memcpy(buf, p, len);
> + buf[len] = '\0';
> + p += len;
> +
> + if (!inet_pton(AF_INET, buf, &addr))
> + return false;
> +
> + if (bracket && !parse_literal(&p, "]"))
> + return false;
> +
> + *cursor = p;
> + *abuf = addr;
> + return true;
> +}
> +
> +/**
> + * parse_ipv6() - Parse an IPv6 address from a string
> + * @abuf: On success, updated with parsed address
> + */
> +static bool parse_ipv6(const char **cursor, struct in6_addr *abuf)
> +{
> + const char *p = *cursor;
> + bool bracket = parse_literal(&p, "[");
> + char buf[INET6_ADDRSTRLEN];
> + struct in6_addr addr;
> + size_t len;
> +
> + if (bracket)
> + len = strcspn(p, "]");
> + else
> + len = strspn(p, "0123456789aAbBcCdDeEfF:.");
> +
> + if (len >= sizeof(buf))
> + return false;
> + memcpy(buf, p, len);
> + buf[len] = '\0';
> + p += len;
> +
> + if (!inet_pton(AF_INET6, buf, &addr))
> + return false;
> +
> + if (bracket && !parse_literal(&p, "]"))
> + return false;
> +
> + *cursor = p;
> + *abuf = addr;
> + return true;
> +}
> +
> +/**
> + * parse_inany() - Parse an IPv4 or IPv6 address from a string
> + * @addr: On success, updated with parsed address
> + */
> +bool parse_inany(const char **cursor, union inany_addr *addr)
> +{
> + struct in_addr a4;
> +
> + if (parse_ipv6(cursor, &addr->a6))
> + return true;
> +
> + if (parse_ipv4(cursor, &a4)) {
> + *addr = inany_from_v4(a4);
> + return true;
> + }
> +
> + return false;
> +}
> diff --git a/parse.h b/parse.h
> index 155b3995..2820a065 100644
> --- a/parse.h
> +++ b/parse.h
> @@ -9,6 +9,8 @@
> #include <stdbool.h>
> #include <netinet/in.h>
>
> +union inany_addr;
> +
> /**
> * port_range() - Represents a non-empty range of ports
> * @first: First port number in the range
> @@ -24,5 +26,7 @@ bool parse_literal(const char **cursor, const char *lit);
> bool parse_eoi(const char *cursor);
> bool parse_unsigned(const char **cursor, int base, unsigned long *valp);
> bool parse_port_range(const char **cursor, struct port_range *range);
> +bool parse_ipv4(const char **cursor, struct in_addr *abuf);
> +bool parse_inany(const char **cursor, union inany_addr *addr);
>
> #endif /* _PARSE_H */
--
Stefano
next prev parent reply other threads:[~2026-07-01 0:07 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-26 7:09 [PATCH 00/12] Rework option parsing in preparation for destination remapping David Gibson
2026-06-26 7:09 ` [PATCH 01/12] Makefile: Add missing PESTO_HEADERS variable David Gibson
2026-07-01 0:07 ` Stefano Brivio
2026-07-01 1:23 ` David Gibson
2026-06-26 7:09 ` [PATCH 02/12] conf: Use parameter instead of global in conf_nat() David Gibson
2026-07-01 0:07 ` Stefano Brivio
2026-07-01 1:24 ` David Gibson
2026-06-26 7:09 ` [PATCH 03/12] parse: Start splitting out parsing helpers David Gibson
2026-07-01 0:07 ` Stefano Brivio
2026-07-01 1:36 ` David Gibson
2026-06-26 7:09 ` [PATCH 04/12] conf: Remove duplicate parsing of -F option David Gibson
2026-06-26 7:09 ` [PATCH 05/12] conf: Clean up conf_ip4_prefix() David Gibson
2026-06-26 7:09 ` [PATCH 06/12] parse: Add helper to parse unsigned integer values David Gibson
2026-06-26 7:09 ` [PATCH 07/12] parse: Move parse_port_range() to new parsing framework David Gibson
2026-06-26 7:09 ` [PATCH 08/12] parse: Add helpers for parsing IP addresses David Gibson
2026-07-01 0:07 ` Stefano Brivio [this message]
2026-07-01 0:09 ` [RESEND] " Stefano Brivio
2026-07-01 0:11 ` [RESEND #2] " Stefano Brivio
2026-07-01 1:44 ` David Gibson
2026-07-01 1:43 ` David Gibson
2026-06-26 7:10 ` [PATCH 09/12] conf: Move address configuration into helper function David Gibson
2026-07-01 0:07 ` Stefano Brivio
2026-07-01 1:44 ` David Gibson
2026-06-26 7:10 ` [PATCH 10/12] conf: Use new parsing tools to handle -a option David Gibson
2026-07-01 0:07 ` Stefano Brivio
2026-07-01 1:55 ` David Gibson
2026-06-26 7:10 ` [PATCH 11/12] fwd_rule: Allow "all" port specs to be combined with other options David Gibson
2026-07-01 0:07 ` Stefano Brivio
2026-07-01 1:55 ` David Gibson
2026-06-26 7:10 ` [PATCH 12/12] fwd_rule: Rewrite forward rule parsing using parse.c helpers David Gibson
2026-07-01 0:08 ` Stefano Brivio
2026-07-01 3:03 ` David Gibson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260701020725.024edf81@elisabeth \
--to=sbrivio@redhat.com \
--cc=david@gibson.dropbear.id.au \
--cc=jamloy@redhat.com \
--cc=passt-dev@passt.top \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).