* [PATCH iproute2 1/1] tc: fix output when printing flower's TOS or TTL @ 2018-03-10 13:31 Roman Mashak 2018-03-10 17:11 ` Stephen Hemminger 0 siblings, 1 reply; 3+ messages in thread From: Roman Mashak @ 2018-03-10 13:31 UTC (permalink / raw) To: stephen; +Cc: netdev, kernel, jhs, jiri, xiyou.wangcong, Roman Mashak 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); } -- 2.7.4 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH iproute2 1/1] tc: fix output when printing flower's TOS or TTL 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 0 siblings, 1 reply; 3+ messages in thread From: Stephen Hemminger @ 2018-03-10 17:11 UTC (permalink / raw) To: Roman Mashak; +Cc: netdev, kernel, jhs, jiri, xiyou.wangcong 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, ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH iproute2 1/1] tc: fix output when printing flower's TOS or TTL 2018-03-10 17:11 ` Stephen Hemminger @ 2018-03-10 18:56 ` Roman Mashak 0 siblings, 0 replies; 3+ messages in thread From: Roman Mashak @ 2018-03-10 18:56 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev, kernel, jhs, jiri, xiyou.wangcong 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. ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-03-10 18:56 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox