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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <857eqjlpyk.fsf@sevai.TCC.com>
Date:   Sat, 10 Mar 2018 13:56:19 -0500
From:   Roman Mashak <mrv@...atatu.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
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

Stephen Hemminger <stephen@...workplumber.org> writes:

> 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,

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.