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

[Stefano Brivio <sbrivio@redhat.com>]

[Dropping redundant email address for David in all the replies to this
series, but it would be nice if you didn't add it in the first place]

On Tue, 24 Mar 2026 10:26:15 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

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

Note: this is now fixed in:

  [PATCH v2] conf: use a single buffer for print formatting in conf_print()
  https://archives.passt.top/passt-dev/20260327195551.271076-1-jmaloy@redhat.com

-- 
Stefano