From: Stefano Brivio <sbrivio@redhat.com>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: passt-dev@passt.top, Jon Maloy <jmaloy@redhat.com>
Subject: [RESEND #2] Re: [PATCH 08/12] parse: Add helpers for parsing IP addresses
Date: Wed, 01 Jul 2026 02:11:52 +0200 (CEST) [thread overview]
Message-ID: <20260701021152.600ca7b0@elisabeth> (raw)
In-Reply-To: <20260701020725.024edf81@elisabeth>
*Actually* resending to jmaloy@redhat.com, the original email had:
On Wed, 1 Jul 2026 02:07:25 +0200
Stefano Brivio <sbrivio@redhat.com> wrote:
> 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>
...jamloy@redhat.com instead.
> >
> > 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:11 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
2026-07-01 0:09 ` [RESEND] " Stefano Brivio
2026-07-01 0:11 ` Stefano Brivio [this message]
2026-07-01 1:44 ` [RESEND #2] " 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=20260701021152.600ca7b0@elisabeth \
--to=sbrivio@redhat.com \
--cc=david@gibson.dropbear.id.au \
--cc=jmaloy@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).