Re: [libnftnl PATCH] rule: Don't append a newline when printing a rule

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Tue, Oct 01, 2024 at 07:45:22PM +0200, Phil Sutter wrote:
> Since commit c759027a526ac, printed rules may or may not end with a
> newline depending on whether userdata was present or not. Deal with this
> inconsistency by avoiding the trailing newline in all cases.

LGTM, thanks Phil.

> Fixes: c759027a526ac ("rule, set_elem: remove trailing \n in userdata snprintf")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> This supersedes the previous patch with subject: Partially revert "rule,
> set_elem: remove trailing \n in userdata snprintf" by solving the
> problem in the opposite direction. As correctly assessed by Pablo, this
> way is consistent with other printers.
> 
> I tested the change with nftables and iptables testsuites. It breaks a
> single test in the latter which compares full ruleset debug output
> against a record. To fix that, one could just pass --ignore-blank-lines
> parameter to the diff call.
> ---
>  src/rule.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/src/rule.c b/src/rule.c
> index 811d5a213f835..c22918a8f3527 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -573,23 +573,21 @@ static int nftnl_rule_snprintf_default(char *buf, size_t remain,
>  		sep = " ";
>  	}
>  
> -	ret = snprintf(buf + offset, remain, "\n");
> -	SNPRINTF_BUFFER_SIZE(ret, remain, offset);
> -
>  	list_for_each_entry(expr, &r->expr_list, head) {
> -		ret = snprintf(buf + offset, remain, "  [ %s ", expr->ops->name);
> +		ret = snprintf(buf + offset, remain,
> +			       "\n  [ %s ", expr->ops->name);
>  		SNPRINTF_BUFFER_SIZE(ret, remain, offset);
>  
>  		ret = nftnl_expr_snprintf(buf + offset, remain, expr,
>  					     type, flags);
>  		SNPRINTF_BUFFER_SIZE(ret, remain, offset);
>  
> -		ret = snprintf(buf + offset, remain, "]\n");
> +		ret = snprintf(buf + offset, remain, "]");
>  		SNPRINTF_BUFFER_SIZE(ret, remain, offset);
>  	}
>  
>  	if (r->user.len) {
> -		ret = snprintf(buf + offset, remain, "  userdata = { ");
> +		ret = snprintf(buf + offset, remain, "\n  userdata = { ");
>  		SNPRINTF_BUFFER_SIZE(ret, remain, offset);
>  
>  		for (i = 0; i < r->user.len; i++) {
> -- 
> 2.43.0
>