Re: [libnftnl PATCH] Partially revert "rule, set_elem: remove trailing \n in userdata snprintf"

[Phil Sutter <phil@nwl.cc>]

On Tue, Oct 01, 2024 at 02:47:01PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Oct 01, 2024 at 02:39:52PM +0200, Phil Sutter wrote:
> > Hi Pablo,
> > 
> > On Tue, Oct 01, 2024 at 01:23:58PM +0200, Pablo Neira Ayuso wrote:
> > > On Tue, Oct 01, 2024 at 01:20:54PM +0200, Phil Sutter wrote:
> > > > This reverts the rule-facing part of commit
> > > > c759027a526ac09ce413dc88c308a4ed98b33416.
> > > > 
> > > > It can't be right: Rules without userdata are printed with a trailing
> > > > newline, so this commit made behaviour inconsistent.
> > > 
> > > Did you run tests/py with this? It is the primary user for this.
> > 
> > It doesn't cover this because there's no test containing a rule with a
> > comment. I just added a respective test, but only to notice it does not
> > matter because nft-test.py compares rules' payload individually per-rule
> > and thus does not care whether output has a trailing newline or not.
> > 
> > I noticed it when testing the iptables compat ext stuff. You can easily
> > reproduce it like so:
> > 
> > | # nft --check --debug=netlink 'table t { chain c { accept comment mycomment; accept; accept;};}'
> > | ip (null) (null) use 0
> > | ip t c
> > |   [ immediate reg 0 accept ]
> > |   userdata = { \x00\x0amycomment\x00 }
> > | ip t c
> > |   [ immediate reg 0 accept ]
> > | 
> > | ip t c
> > |   [ immediate reg 0 accept ]
> > 
> > Note the missing empty line after the first rule.
> 
> None of the existing libnftnl _snprintf functions terminate string
> with line break, right?. I mean, for consistency with other existing
> _snprintf functions. Maybe fix nft instead?

Correct, though others don't emit newlines at all. I'll change rule
debug output to never append a newline and see what breaks.

Thanks, Phil