netdev - Re: Question Print Formatting iproute2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <03088b77a44bbe9694ad3b337d0106b1df3fd449.camel@gmail.com>
Date:   Mon, 27 Jul 2020 21:12:54 -0700
From:   Briana Oursler <briana.oursler@...il.com>
To:     Petr Machata <petrm@...lanox.com>
Cc:     netdev@...r.kernel.org,
        Stephen Hemminger <stephen@...workplumber.org>,
        Alexey Kuznetsov <kuznet@....inr.ac.ru>,
        Davide Caratti <dcaratti@...hat.com>,
        Stefano Brivio <sbrivio@...hat.com>,
        Jiri Pirko <jiri@...lanox.com>
Subject: Re: Question Print Formatting iproute2

On Mon, 2020-07-27 at 21:31 +0200, Petr Machata wrote:
> Briana Oursler <briana.oursler@...il.com> writes:
> 
> > 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.
> 
> Yeah, I outsmarted myself with those space changes. Those two chunks
> need reversing, and qevents need to have the space changed. This
> should
> work:
> 
Thank you for the response. I see what you are saying. I had not seen
the qevents, I'll put all 3 in.

> modified	  tc/q_red.c
> @@ -222,12 +222,12 @@ 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);
> 
>  	if (show_details) {
> -		print_uint(PRINT_ANY, "ewma", " ewma %u ", qopt->Wlog);
> +		print_uint(PRINT_ANY, "ewma", "ewma %u ", qopt->Wlog);
>  		if (max_P)
>  			print_float(PRINT_ANY, "probability",
>  				    "probability %lg ", max_P / pow(2,
> 32));
> modified	  tc/tc_qevent.c
> @@ -82,8 +82,9 @@ void qevents_print(struct qevent_util *qevents,
> FILE *f)
>  			}
> 
>  			open_json_object(NULL);
> -			print_string(PRINT_ANY, "kind", " qevent %s",
> qevents->id);
> +			print_string(PRINT_ANY, "kind", "qevent %s",
> qevents->id);
>  			qevents->print_qevent(qevents, f);
> +			print_string(PRINT_FP, NULL, "%s", " ");
>  			close_json_object();
>  		}
>  	}
> 
> Are you going to take care of this, or should I?

I will, I'll amend the commit I included so it will have the other
changes you suggest and send as a regular patch.