From: Stephen Hemminger <stephen@networkplumber•org>
To: Petr Machata <me@pmachata•org>
Cc: netdev@vger•kernel.org, dsahern@gmail•com, Po.Liu@nxp•com,
toke@toke•dk, dave.taht@gmail•com, edumazet@google•com,
tahiliani@nitk•edu.in, vtlam@google•com, leon@kernel•org
Subject: Re: [PATCH iproute2-next 2/6] lib: Move print_rate() from tc here; modernize
Date: Mon, 30 Nov 2020 16:35:47 -0800 [thread overview]
Message-ID: <20201130163547.23a06e79@hermes.local> (raw)
In-Reply-To: <f2dd583ef64b64b95571b317c94802ff155ebc5d.1606774951.git.me@pmachata.org>
On Mon, 30 Nov 2020 23:59:38 +0100
Petr Machata <me@pmachata•org> wrote:
> The functions print_rate() and sprint_rate() are useful for formatting
> rate-like values. The DCB tool would find these useful in the maxrate
> subtool. However, the current interface to these functions uses a global
> variable use_iec as a flag indicating whether 1024- or 1000-based powers
> should be used when formatting the rate value. For general use, a global
> variable is not a great way of passing arguments to a function. Besides, it
> is unlike most other printing functions in that it deals in buffers and
> ignores JSON.
>
> Therefore make the interface to print_rate() explicit by converting use_iec
> to an ordinary parameter. Since the interface changes anyway, convert it to
> follow the pattern of other json_print functions (except for the
> now-explicit use_iec parameter). Move to json_print.c.
>
> Add a wrapper to tc, so that all the call sites do not need to repeat the
> use_iec global variable argument, and convert all call sites.
>
> In q_cake.c, the conversion is not straightforward due to usage of a macro
> that is shared across numerous data types. Simply hand-roll the
> corresponding code, which seems better than making an extra helper for one
> call site.
>
> Drop sprint_rate() now that everybody just uses print_rate().
>
> Signed-off-by: Petr Machata <me@pmachata•org>
> ---
> include/json_print.h | 10 ++++++++++
> lib/json_print.c | 32 ++++++++++++++++++++++++++++++++
> tc/m_police.c | 9 ++++-----
> tc/q_cake.c | 28 ++++++++++++++++------------
> tc/q_cbq.c | 14 ++++----------
> tc/q_fq.c | 25 +++++++++----------------
> tc/q_hfsc.c | 4 ++--
> tc/q_htb.c | 4 ++--
> tc/q_mqprio.c | 8 ++++----
> tc/q_netem.c | 4 +---
> tc/q_tbf.c | 7 ++-----
> tc/tc_util.c | 37 +++++++------------------------------
> tc/tc_util.h | 4 ++--
> 13 files changed, 95 insertions(+), 91 deletions(-)
>
> diff --git a/include/json_print.h b/include/json_print.h
> index 096a999a4de4..b6c4c0c80833 100644
> --- a/include/json_print.h
> +++ b/include/json_print.h
> @@ -86,4 +86,14 @@ _PRINT_NAME_VALUE_FUNC(uint, unsigned int, u);
> _PRINT_NAME_VALUE_FUNC(string, const char*, s);
> #undef _PRINT_NAME_VALUE_FUNC
>
> +int print_color_rate(bool use_iec, enum output_type t, enum color_attr color,
> + const char *key, const char *fmt, unsigned long long rate);
> +
> +static inline int print_rate(bool use_iec, enum output_type t,
> + const char *key, const char *fmt,
> + unsigned long long rate)
> +{
> + return print_color_rate(use_iec, t, COLOR_NONE, key, fmt, rate);
> +}
> +
Overall this looks good, but is there any case where color output
makes sense for this field? If not then why do all the color wrappers.
next prev parent reply other threads:[~2020-12-01 0:36 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-30 22:59 [PATCH iproute2-next 0/6] Move rate and size parsing and output to lib Petr Machata
2020-11-30 22:59 ` [PATCH iproute2-next 1/6] Move the use_iec declaration to the tools Petr Machata
2020-11-30 22:59 ` [PATCH iproute2-next 2/6] lib: Move print_rate() from tc here; modernize Petr Machata
2020-12-01 0:35 ` Stephen Hemminger [this message]
2020-12-01 22:56 ` Petr Machata
2020-11-30 22:59 ` [PATCH iproute2-next 3/6] lib: Move sprint_size() from tc here, add print_size() Petr Machata
2020-12-01 0:39 ` Stephen Hemminger
2020-12-01 22:41 ` Petr Machata
2020-12-02 4:07 ` David Ahern
2020-11-30 22:59 ` [PATCH iproute2-next 4/6] lib: Move get_rate(), get_rate64() from tc here Petr Machata
2020-11-30 22:59 ` [PATCH iproute2-next 5/6] lib: Move get_size() " Petr Machata
2020-11-30 22:59 ` [PATCH iproute2-next 6/6] lib: print_rate(): Fix formatting small rates in IEC mode Petr Machata
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201130163547.23a06e79@hermes.local \
--to=stephen@networkplumber$(echo .)org \
--cc=Po.Liu@nxp$(echo .)com \
--cc=dave.taht@gmail$(echo .)com \
--cc=dsahern@gmail$(echo .)com \
--cc=edumazet@google$(echo .)com \
--cc=leon@kernel$(echo .)org \
--cc=me@pmachata$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=tahiliani@nitk$(echo .)edu.in \
--cc=toke@toke$(echo .)dk \
--cc=vtlam@google$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox