Question Print Formatting iproute2

* Question Print Formatting iproute2
@ 2020-07-27  4:46 Briana Oursler
  2020-07-27 19:31 ` Petr Machata
  2020-07-27 23:47 ` Stephen Hemminger
  0 siblings, 2 replies; 11+ messages in thread
From: Briana Oursler @ 2020-07-27  4:46 UTC (permalink / raw)
  To: netdev
  Cc: Stephen Hemminger, Alexey Kuznetsov, Petr Machata,
	Davide Caratti, Stefano Brivio, Jiri Pirko, Briana Oursler

I have a patch I've written to address a format specifier change that
breaks some tests in tc-testing, but I wanted to ask about the change
and for some guidance with respect to how formatters are approached in
iproute2. 

On a recent run of tdc tests I ran ./tdc.py -c qdisc and found:

1..91
not ok 1 8b6e - Create RED with no flags
        Could not match regex pattern. Verify command output:
qdisc red 1: root refcnt 2 limit 1Mb min 100Kb max 300Kb

not ok 2 342e - Create RED with adaptive flag
        Could not match regex pattern. Verify command output:
qdisc red 1: root refcnt 2 limit 1Mb min 100Kb max 300Kbadaptive

not ok 3 2d4b - Create RED with ECN flag
        Could not match regex pattern. Verify command output:
qdisc red 1: root refcnt 2 limit 1Mb min 100Kb max 300Kbecn

not ok 4 650f - Create RED with flags ECN, adaptive
        Could not match regex pattern. Verify command output:
qdisc red 1: root refcnt 2 limit 1Mb min 100Kb max 300Kbecn adaptive

not ok 5 5f15 - Create RED with flags ECN, harddrop
        Could not match regex pattern. Verify command output:
qdisc red 1: root refcnt 2 limit 1Mb min 100Kb max 300Kbecn harddrop

not ok 6 53e8 - Create RED with flags ECN, nodrop
        Could not match regex pattern. Verify command output:
qdisc red 1: root refcnt 2 limit 1Mb min 100Kb max 300Kbecn nodrop

ok 7 d091 - Fail to create RED with only nodrop flag
not ok 8 af8e - Create RED with flags ECN, nodrop, harddrop
        Could not match regex pattern. Verify command output:
qdisc red 1: root refcnt 2 limit 1Mb min 100Kb max 300Kbecn harddrop
nodrop

I git bisected and found d0e450438571("tc: q_red: Add support for
qevents "mark" and "early_drop"), the commit that introduced the
formatting change causing the break. 

-       print_string(PRINT_FP, NULL, "max %s ", sprint_size(qopt->qth_max, b3));
+       print_string(PRINT_FP, NULL, "max %s", sprint_size(qopt->qth_max, b3));

I made a patch that adds a space after the format specifier in the
iproute2 tc/q_red.c and tested it using: tdc.py -c qdisc. After the
change, all the broken tdc qdisc red tests return ok. I'm including the
patch under the scissors line.

I wanted to ask the ML if adding the space after the specifier is preferred usage.
The commit also had: 
 -               print_uint(PRINT_ANY, "ewma", "ewma %u ", qopt->Wlog);
 +               print_uint(PRINT_ANY, "ewma", " ewma %u ", qopt->Wlog);

so I wanted to check with everyone.

Thanks 
>8------------------------------------------------------------------------8<
From 1e7bee22a799a320bd230ad959d459b386bec26d Mon Sep 17 00:00:00 2001
Subject: [RFC iproute2-next] tc: Add space after format specifier

Add space after format specifier in print_string call. Fixes broken
qdisc tests within tdc testing suite.

Fixes: d0e450438571("tc: q_red: Add support for
qevents "mark" and "early_drop")

Signed-off-by: Briana Oursler <briana.oursler@gmail.com>
---
 tc/q_red.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tc/q_red.c b/tc/q_red.c
index dfef1bf8..7106645a 100644
--- a/tc/q_red.c
+++ b/tc/q_red.c
@@ -222,7 +222,7 @@ static int red_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 	print_uint(PRINT_JSON, "min", NULL, qopt->qth_min);
 	print_string(PRINT_FP, NULL, "min %s ", sprint_size(qopt->qth_min, b2));
 	print_uint(PRINT_JSON, "max", NULL, qopt->qth_max);
-	print_string(PRINT_FP, NULL, "max %s", sprint_size(qopt->qth_max, b3));
+	print_string(PRINT_FP, NULL, "max %s ", sprint_size(qopt->qth_max, b3));
 
 	tc_red_print_flags(qopt->flags);
 

base-commit: 1ca65af1c5e131861a3989cca3c7ca8b067e0833
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread