Re: [PATCH iptables-nft 1/3] xlate: get rid of escape_quotes

[Florian Westphal <fw@strlen.de>]

Phil Sutter <phil@nwl.cc> wrote:
> On Thu, Nov 24, 2022 at 02:49:37PM +0100, Florian Westphal wrote:
> > Its not necessary to escape " characters, we can simply
> > let xtables-translate print the entire translation/command
> > enclosed in '' chracters, i.e. nft 'add rule ...', this also takes
> > care of [, { and other special characters that some shells might
> > parse otherwise (when copy-pasting translated output).
> > 
> > This breaks all xlate test cases, fixup in followup patches.
> > 
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > ---
> [...]
> > diff --git a/include/xtables.h b/include/xtables.h
> > index 9eba4f619d35..150d40bfafd9 100644
> > --- a/include/xtables.h
> > +++ b/include/xtables.h
> > @@ -211,14 +211,12 @@ struct xt_xlate_mt_params {
> >  	const void			*ip;
> >  	const struct xt_entry_match	*match;
> >  	int				numeric;
> > -	bool				escape_quotes;
> >  };
> >  
> >  struct xt_xlate_tg_params {
> >  	const void			*ip;
> >  	const struct xt_entry_target	*target;
> >  	int				numeric;
> > -	bool				escape_quotes;
> >  };
> 
> Does this break ABI compatibility?

Yes.  I can keep the bool as a dead member if you prefer.

> >  	if (ret)
> > -		printf("%s\n", xt_xlate_get(xl));
> > +		printf("%s", xt_xlate_get(xl));
> >  
> > +	puts("'");
> >  	xt_xlate_free(xl);
> >  	return ret;
> >  }
> 
> If h->ops->xlate() fails, the code prints "'\n". How about:
> 
> | if (ret)
> | 	printf("%s'\n", xt_xlate_get(xl));
> 
> Or am I missing something?

We already printed 'insert rule, hence it was weird for the '\n' to be missed, but I see that
the caller will print a ' # iptables-syntax' in that case, so I will
re-add the '\n' to where it was.

> >  	if (set[0]) {
> > -		printf("add set %s %s %s\n", family2str[h->family], p->table,
> > +		printf("'add set %s %s %s'\n", family2str[h->family], p->table,
> >  		       xt_xlate_set_get(xl));
> 
> Quoting needs to respect cs->restore value, no? Maybe simpler to
> introduce 'const char *tick = cs->restore ? "" : "'";' and just insert
> it everywhere needed.

Will do that.