From: Roman Mashak <mrv@mojatatu•com>
To: Stephen Hemminger <stephen@networkplumber•org>
Cc: netdev@vger•kernel.org, kernel@mojatatu•com, jhs@mojatatu•com,
jiri@resnulli•us, xiyou.wangcong@gmail•com
Subject: Re: [PATCH iproute2 1/1] tc: fix output when printing flower's TOS or TTL
Date: Sat, 10 Mar 2018 13:56:19 -0500 [thread overview]
Message-ID: <857eqjlpyk.fsf@sevai.TCC.com> (raw)
In-Reply-To: <20180310091149.47f8e9f1@xeon-e3> (Stephen Hemminger's message of "Sat, 10 Mar 2018 09:11:49 -0800")
Stephen Hemminger <stephen@networkplumber•org> writes:
> On Sat, 10 Mar 2018 08:31:26 -0500
> Roman Mashak <mrv@mojatatu•com> wrote:
>
>> Signed-off-by: Roman Mashak <mrv@mojatatu•com>
>> ---
>> tc/f_flower.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tc/f_flower.c b/tc/f_flower.c
>> index 5a4ec832bc19..5cf19139dcae 100644
>> --- a/tc/f_flower.c
>> +++ b/tc/f_flower.c
>> @@ -1131,7 +1131,7 @@ static void flower_print_ip_attr(char *name, struct rtattr *key_attr,
>> if (mask_attr)
>> sprintf(out + done, "/%x", rta_getattr_u8(mask_attr));
>>
>> - sprintf(namefrm, "\n %s %%x", name);
>> + sprintf(namefrm, "\n %s %%s", name);
>> print_string(PRINT_ANY, name, namefrm, out);
>> }
>>
>
> I don't see why code has to build a format string here, and not do what other code does
> and do multiple print's. Plus for JSON it is better to not create aggregated types.
>
> Also sprintf() should not be used only snprintf.
>
> And the flower code seems to have forgotten the seldom used oneline mode.
>
> Maybe something like:
>
> -static void flower_print_ip_attr(char *name, struct rtattr *key_attr,
> +static void flower_print_ip_attr(const char *name, struct rtattr *key_attr,
> struct rtattr *mask_attr)
> {
> - SPRINT_BUF(namefrm);
> - SPRINT_BUF(out);
> - size_t done;
> + SPRINT_BUF(mask_name);
>
> if (!key_attr)
> return;
>
> - done = sprintf(out, "%x", rta_getattr_u8(key_attr));
> - if (mask_attr)
> - sprintf(out + done, "/%x", rta_getattr_u8(mask_attr));
> + print_string(PRINT_FP, NULL, "%s ", _SL_);
> + print_string(PRINT_FP, NULL, "%s ", name);
> + print_0xhex(PRINT_ANY, name, "%x", rta_getattr_u8(key_attr));
>
> - sprintf(namefrm, "\n %s %%x", name);
> - print_string(PRINT_ANY, name, namefrm, out);
> + if (mask_attr) {
> + snprintf(mask_name, sizeof(mask_name),
> + "%s_mask", name);
> + print_0xhex(PRINT_ANY, mask_name, "/%x",
> + rta_getattr_u8(mask_attr));
> + }
> }
>
> static void flower_print_matching_flags(char *name,
Hi Stephen, thanks for suggestion. Would you submit your patch, or
should I prepare v2? Also, 'oneline' mode isn't supported in tc at the
moment, so needs to be added for consistency with ip or bridge.
prev parent reply other threads:[~2018-03-10 18:56 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-10 13:31 [PATCH iproute2 1/1] tc: fix output when printing flower's TOS or TTL Roman Mashak
2018-03-10 17:11 ` Stephen Hemminger
2018-03-10 18:56 ` Roman Mashak [this message]
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=857eqjlpyk.fsf@sevai.TCC.com \
--to=mrv@mojatatu$(echo .)com \
--cc=jhs@mojatatu$(echo .)com \
--cc=jiri@resnulli$(echo .)us \
--cc=kernel@mojatatu$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=stephen@networkplumber$(echo .)org \
--cc=xiyou.wangcong@gmail$(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