Re: [PATCH 09/12] conf: Move address configuration into helper function

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 5268 bytes --]

On Wed, Jul 01, 2026 at 02:07:38AM +0200, Stefano Brivio wrote:
> On Fri, 26 Jun 2026 17:10:00 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > The handling of the -a option is getting complex enough that it's pretty
> > bulky inside it's switch label.  Move it into a new conf_addr() function.
> 
> Nit if you respin: 'its'.

Done.

> 
> > We also rename the bulky addr_has_prefix_len and prefix_len_from_opt
> > variables to the terser 'opt_a_is_prefix' and 'opt_n', since they are
> > specifically about those command line options.
> > 
> > Cc: Jon Maloy <jmaloy@redhat.com>
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  conf.c | 96 ++++++++++++++++++++++++++++++++--------------------------
> >  1 file changed, 53 insertions(+), 43 deletions(-)
> > 
> > diff --git a/conf.c b/conf.c
> > index 2d1895d4..3614776c 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -1168,6 +1168,53 @@ int conf_tap_fd(const char *arg)
> >  	return val;
> >  }
> >  
> > +/**
> > + * conf_addr() - Configure guest address with -a option
> > + * @c:		Execution context
> > + * @arg:	-a command line argument
> > + * @opt_n:	Value from -n option, if any
> > + */
> > +static bool conf_addr(struct ctx *c, char *arg, uint8_t opt_n)
> > +{
> > +	union inany_addr addr;
> > +	const char *p = arg;
> > +	uint8_t prefix_len;
> > +	bool is_prefix;
> > +
> > +	is_prefix = inany_prefix_pton(arg, &addr, &prefix_len);
> > +
> > +	if (is_prefix && opt_n)
> > +		die("Redundant prefix length specification");
> > +
> > +	if (!is_prefix &&
> > +	    !(parse_inany(&p, &addr) && parse_eoi(p)))
> > +		die("Invalid address: %s", arg);
> > +
> > +	if (opt_n && inany_v4(&addr))
> > +		prefix_len = opt_n;
> > +	else if (!is_prefix)
> > +		prefix_len = inany_default_prefix_len(&addr);
> > +
> > +	if (inany_is_unspecified(&addr) || inany_is_multicast(&addr) ||
> > +	    inany_is_loopback(&addr) || IN6_IS_ADDR_V4COMPAT(&addr.a6))
> > +		die("Invalid address: %s", arg);
> > +
> > +	if (inany_v4(&addr)) {
> > +		c->ip4.addr = *inany_v4(&addr);
> > +		c->ip4.prefix_len = prefix_len - 96;
> > +		c->ip4.addr_fixed = true;
> > +		if (c->mode == MODE_PASTA)
> > +			c->ip4.no_copy_addrs = true;
> > +	} else {
> > +		c->ip6.addr = addr.a6;
> > +		c->ip6.addr_fixed = true;
> > +		if (c->mode == MODE_PASTA)
> > +			c->ip6.no_copy_addrs = true;
> > +	}
> > +
> > +	return is_prefix;
> > +}
> > +
> >  /**
> >   * conf() - Process command-line arguments and set configuration
> >   * @c:		Execution context
> > @@ -1262,12 +1309,12 @@ void conf(struct ctx *c, int argc, char **argv)
> >  	unsigned dns4_idx = 0, dns6_idx = 0;
> >  	unsigned long max_mtu = IP_MAX_MTU;
> >  	struct fqdn *dnss = c->dns_search;
> > -	bool addr_has_prefix_len = false;
> > -	uint8_t prefix_len_from_opt = 0;
> >  	unsigned int ifi4 = 0, ifi6 = 0;
> > +	bool opt_a_is_prefix = false;
> >  	const char *logfile = NULL;
> >  	char *runas = NULL;
> >  	size_t logsize = 0;
> > +	uint8_t opt_n = 0;
> >  	int name, ret;
> >  	uid_t uid;
> >  	gid_t gid;
> > @@ -1567,52 +1614,15 @@ void conf(struct ctx *c, int argc, char **argv)
> >  			c->mtu = mtu;
> >  			break;
> >  		}
> > -		case 'a': {
> > -			union inany_addr addr;
> > -			uint8_t prefix_len;
> > -
> > -			addr_has_prefix_len = inany_prefix_pton(optarg, &addr,
> > -								&prefix_len);
> > -
> > -			if (addr_has_prefix_len && prefix_len_from_opt)
> > -				die("Redundant prefix length specification");
> > -
> > -			p = optarg;
> > -			if (!addr_has_prefix_len &&
> > -			    !(parse_inany(&p, &addr) && parse_eoi(p)))
> > -				die("Invalid address: %s", optarg);
> > -
> > -			if (prefix_len_from_opt && inany_v4(&addr))
> > -				prefix_len = prefix_len_from_opt;
> > -			else if (!addr_has_prefix_len)
> > -				prefix_len = inany_default_prefix_len(&addr);
> > -
> > -			if (inany_is_unspecified(&addr) ||
> > -			    inany_is_multicast(&addr) ||
> > -			    inany_is_loopback(&addr) ||
> > -			    IN6_IS_ADDR_V4COMPAT(&addr.a6))
> > -				die("Invalid address: %s", optarg);
> > -
> > -			if (inany_v4(&addr)) {
> > -				c->ip4.addr = *inany_v4(&addr);
> > -				c->ip4.prefix_len = prefix_len - 96;
> > -				c->ip4.addr_fixed = true;
> > -				if (c->mode == MODE_PASTA)
> > -					c->ip4.no_copy_addrs = true;
> > -			} else {
> > -				c->ip6.addr = addr.a6;
> > -				c->ip6.addr_fixed = true;
> > -				if (c->mode == MODE_PASTA)
> > -					c->ip6.no_copy_addrs = true;
> > -			}
> > +		case 'a':
> > +			opt_a_is_prefix = conf_addr(c, optarg, opt_n);
> >  			break;
> > -		}
> >  		case 'n':
> > -			if (addr_has_prefix_len)
> > +			if (opt_a_is_prefix)
> >  				die("Redundant prefix length specification");
> >  
> >  			c->ip4.prefix_len = conf_ip4_prefix(optarg);
> > -			prefix_len_from_opt = c->ip4.prefix_len + 96;
> > +			opt_n = c->ip4.prefix_len + 96;
> >  			break;
> >  		case 'M':
> >  			parse_mac(c->our_tap_mac, optarg);
> 
> -- 
> Stefano
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]