From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f196.google.com ([209.85.192.196]:45203 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750884AbeCJRL5 (ORCPT ); Sat, 10 Mar 2018 12:11:57 -0500 Received: by mail-pf0-f196.google.com with SMTP id h19so2572053pfd.12 for ; Sat, 10 Mar 2018 09:11:57 -0800 (PST) Date: Sat, 10 Mar 2018 09:11:49 -0800 From: Stephen Hemminger To: Roman Mashak 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 Message-ID: <20180310091149.47f8e9f1@xeon-e3> In-Reply-To: <1520688686-15966-1-git-send-email-mrv@mojatatu.com> References: <1520688686-15966-1-git-send-email-mrv@mojatatu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org List-ID: On Sat, 10 Mar 2018 08:31:26 -0500 Roman Mashak wrote: > Signed-off-by: Roman Mashak > --- > 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,