Re: [PATCH v6 01/13] conf: use a single buffer for print formatting in conf_print()

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

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

On Sat, Mar 21, 2026 at 08:43:21PM -0400, Jon Maloy wrote:
> The function conf_print() uses three different buffers as target
> for address print formatting. This is unnecessary, as a single
> buffer of length INET6_ADDRSTRLEN has sufficient space for all
> address types, IPv4, IPv6 and MAC. There is no risk for conflicts,
> since all formatting is followed by an immediate info() printout.
> 
> To make our life easier in the following commits, we do this
> simplification here.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> ---
>  conf.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index dafac46..9bcd9de 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -1136,11 +1136,12 @@ enum passt_modes conf_mode(int argc, char *argv[])
>   */
>  static void conf_print(const struct ctx *c)
>  {
> -	char buf4[INET_ADDRSTRLEN], buf6[INET6_ADDRSTRLEN];
> -	char bufmac[ETH_ADDRSTRLEN], ifn[IFNAMSIZ];
> +	char buf[INET6_ADDRSTRLEN];

INANY_ADDRSTRLEN is already defined to be the max of INET_ADDRSTRLEN
and INET6_ADDRSTRLEN.

>  	int i;
>  
>  	if (c->ifi4 > 0 || c->ifi6 > 0) {
> +		char ifn[IFNAMSIZ];
> +
>  		info("Template interface: %s%s%s%s%s",
>  		     c->ifi4 > 0 ? if_indextoname(c->ifi4, ifn) : "",
>  		     c->ifi4 > 0 ? " (IPv4)" : "",
> @@ -1162,24 +1163,24 @@ static void conf_print(const struct ctx *c)
>  	    !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_out)) {
>  		info("Outbound address: %s%s%s",
>  		     IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr_out) ? "" :
> -		     inet_ntop(AF_INET, &c->ip4.addr_out, buf4, sizeof(buf4)),
> +		     inet_ntop(AF_INET, &c->ip4.addr_out, buf, sizeof(buf)),
>  		     (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr_out) &&
>  		      !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_out)) ? ", " : "",
>  		     IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_out) ? "" :
> -		     inet_ntop(AF_INET6, &c->ip6.addr_out, buf6, sizeof(buf6)));
> +		     inet_ntop(AF_INET6, &c->ip6.addr_out, buf, sizeof(buf)));

This one won't work: you're using the same buffer twice for the same
print.  The buffer will be overwritten by the second inet_ntop()
before the results from the first one are printed.

>  	}
>  
>  	if (c->mode == MODE_PASTA && !c->splice_only)
>  		info("Namespace interface: %s", c->pasta_ifn);
>  
>  	info("MAC:");
> -	info("    host: %s", eth_ntop(c->our_tap_mac, bufmac, sizeof(bufmac)));
> +	info("    host: %s", eth_ntop(c->our_tap_mac, buf, sizeof(buf)));
>  
>  	if (c->ifi4) {
>  		if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_host_loopback))
>  			info("    NAT to host 127.0.0.1: %s",
>  			     inet_ntop(AF_INET, &c->ip4.map_host_loopback,
> -				       buf4, sizeof(buf4)));
> +				       buf, sizeof(buf)));
>  
>  		if (!c->no_dhcp) {
>  			uint32_t mask;
> @@ -1188,12 +1189,12 @@ static void conf_print(const struct ctx *c)
>  
>  			info("DHCP:");
>  			info("    assign: %s",
> -			     inet_ntop(AF_INET, &c->ip4.addr, buf4, sizeof(buf4)));
> +			     inet_ntop(AF_INET, &c->ip4.addr, buf, sizeof(buf)));
>  			info("    mask: %s",
> -			     inet_ntop(AF_INET, &mask,        buf4, sizeof(buf4)));
> +			     inet_ntop(AF_INET, &mask,        buf, sizeof(buf)));
>  			info("    router: %s",
>  			     inet_ntop(AF_INET, &c->ip4.guest_gw,
> -				       buf4, sizeof(buf4)));
> +				       buf, sizeof(buf)));
>  		}
>  
>  		for (i = 0; i < ARRAY_SIZE(c->ip4.dns); i++) {
> @@ -1201,8 +1202,8 @@ static void conf_print(const struct ctx *c)
>  				break;
>  			if (!i)
>  				info("DNS:");
> -			inet_ntop(AF_INET, &c->ip4.dns[i], buf4, sizeof(buf4));
> -			info("    %s", buf4);
> +			inet_ntop(AF_INET, &c->ip4.dns[i], buf, sizeof(buf));
> +			info("    %s", buf);
>  		}
>  
>  		for (i = 0; *c->dns_search[i].n; i++) {
> @@ -1216,7 +1217,7 @@ static void conf_print(const struct ctx *c)
>  		if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback))
>  			info("    NAT to host ::1: %s",
>  			     inet_ntop(AF_INET6, &c->ip6.map_host_loopback,
> -				       buf6, sizeof(buf6)));
> +				       buf, sizeof(buf)));
>  
>  		if (!c->no_ndp && !c->no_dhcpv6)
>  			info("NDP/DHCPv6:");
> @@ -1228,12 +1229,12 @@ static void conf_print(const struct ctx *c)
>  			goto dns6;
>  
>  		info("    assign: %s",
> -		     inet_ntop(AF_INET6, &c->ip6.addr, buf6, sizeof(buf6)));
> +		     inet_ntop(AF_INET6, &c->ip6.addr, buf, sizeof(buf)));
>  		info("    router: %s",
> -		     inet_ntop(AF_INET6, &c->ip6.guest_gw, buf6, sizeof(buf6)));
> +		     inet_ntop(AF_INET6, &c->ip6.guest_gw, buf, sizeof(buf)));
>  		info("    our link-local: %s",
>  		     inet_ntop(AF_INET6, &c->ip6.our_tap_ll,
> -			       buf6, sizeof(buf6)));
> +			       buf, sizeof(buf)));
>  
>  dns6:
>  		for (i = 0; i < ARRAY_SIZE(c->ip6.dns); i++) {
> @@ -1241,8 +1242,8 @@ dns6:
>  			    break;
>  			if (!i)
>  				info("DNS:");
> -			inet_ntop(AF_INET6, &c->ip6.dns[i], buf6, sizeof(buf6));
> -			info("    %s", buf6);
> +			inet_ntop(AF_INET6, &c->ip6.dns[i], buf, sizeof(buf));
> +			info("    %s", buf);
>  		}
>  
>  		for (i = 0; *c->dns_search[i].n; i++) {
> -- 
> 2.52.0
> 

-- 
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 --]