netdev - Re: [PATCH iproute2 1/1] tc: fix output when printing flower's TOS or TTL

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 for Android: free password hash cracker in your pocket

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

Date:   Sat, 10 Mar 2018 09:11:49 -0800
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     Roman Mashak <mrv@...atatu.com>
Cc:     netdev@...r.kernel.org, kernel@...atatu.com, jhs@...atatu.com,
        jiri@...nulli.us, xiyou.wangcong@...il.com
Subject: Re: [PATCH iproute2 1/1] tc: fix output when printing flower's TOS
 or TTL

On Sat, 10 Mar 2018 08:31:26 -0500
Roman Mashak <mrv@...atatu.com> wrote:

> Signed-off-by: Roman Mashak <mrv@...atatu.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,