| 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
| ||
|
Message-ID: <87io2qefcf.fsf@rasmusvillemoes.dk>
Date: Mon, 18 Jan 2016 22:40:32 +0100
From: Rasmus Villemoes <linux@...musvillemoes.dk>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Robert Elliott <elliott@....com>,
Matt Fleming <matt@...eblueprint.co.uk>,
Andrew Morton <akpm@...ux-foundation.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org,
Brian Norris <computersforpeace@...il.com>,
Hariprasad S <hariprasad@...lsio.com>
Subject: Re: [PATCH v2 08/11] lib/vsprintf: allow range of prefix for %pl[From[To]]
On Thu, Jan 14 2016, Andy Shevchenko <andriy.shevchenko@...ux.intel.com> wrote:
> The user may supply the range of desired prefixes in a format %pl[From[To]],
> where 'From' is letter of minimum allowed prefix, and 'To' is a maximum
> correspondently.
>
Sorry, but yuck. I really don't think a %p extension is a good way to
achieve this pretty-printing (with or without the min/max unit thing
added). Also, this makes it too easy to lose information (since %plX
would round down to X), so we'd neither get something representing the
passed-in value nor something human readable (as your test case { .v =
1097364144125ULL, .r = "1071644671 KiB" } shows).
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> ---
> Documentation/printk-formats.txt | 4 ++++
> lib/test_printf.c | 35 ++++++++++++++++++++++++++++++++---
> lib/vsprintf.c | 18 +++++++++++++++++-
> 3 files changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
> index 8e8b4c0..3b41899 100644
> --- a/Documentation/printk-formats.txt
> +++ b/Documentation/printk-formats.txt
> @@ -289,9 +289,13 @@ struct clk:
>
> unsigned long long value in human-readable form (with IEC binary prefix):
>
> + %pl[From[To]]
> +
> %pl 1023 KiB (1047552)
> %pl 1048575 B (1048575)
> + %plKM 1023 KiB (1048575)
> %pl 1 MiB (1048576)
> + %plG 2 TiB (2199023255552)
>
> For printing given unsigned long long value in human-readable form with
> automatically choosen IEC binary prefix.
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 1e078c2..ae35885 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -420,10 +420,39 @@ human_readable(void)
> { .v = 1048575ULL, .r = "1048575 B" },
> { .v = 1047552ULL, .r = "1023 KiB" },
> };
> - unsigned int i;
> + struct hr_test_data htdplM[] = {
> + { .v = 1097364144128ULL, .r = "1022 GiB" },
> + { .v = 1097364144125ULL, .r = "1046527 MiB" },
> + { .v = 1048576ULL, .r = "1 MiB" },
> + { .v = 1048575ULL, .r = "0 MiB" },
> + { .v = 1047552ULL, .r = "0 MiB" },
> + };
> + struct hr_test_data htdplKM[] = {
> + { .v = 1097364144128ULL, .r = "1046528 MiB" },
> + { .v = 1097364144125ULL, .r = "1071644671 KiB" },
> + { .v = 1048578ULL, .r = "1024 KiB" },
> + { .v = 1048576ULL, .r = "1 MiB" },
> + { .v = 1048575ULL, .r = "1023 KiB" },
> + { .v = 1047552ULL, .r = "1023 KiB" },
> + };
> + struct hr_test_array {
> + const char *fmt;
> + struct hr_test_data *data;
> + size_t sz;
> + };
> + struct hr_test_array hta[] = {
> + { "%pl", htdpl, ARRAY_SIZE(htdpl) },
> + { "%plM", htdplM, ARRAY_SIZE(htdplM) },
> + { "%plKM", htdplKM, ARRAY_SIZE(htdplKM) },
> + /* No reversed %pl[To[From]] order */
> + { "%plMK", htdplM, ARRAY_SIZE(htdplM) },
> + };
> + unsigned int i, j;
>
> - for (i = 0; i < ARRAY_SIZE(htdpl); i++)
> - test(htdpl[i].r, "%pl", &htdpl[i].v);
> + for (j = 0; j < ARRAY_SIZE(hta); j++) {
> + for (i = 0; i < hta[j].sz; i++)
> + test(hta[j].data[i].r, hta[j].fmt, &hta[j].data[i].v);
> + }
> }
>
> static void __init
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 8288470..fbcb221 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1310,12 +1310,28 @@ char *human_readable(char *buf, char *end, const void *addr,
> struct printf_spec spec, const char *fmt)
> {
> unsigned long long value = *(unsigned long long *)addr;
> + unsigned short from = 0, to = STRING_UNITS_2_NUM - 1;
> unsigned long index;
> + unsigned int i;
> +
Is there any reason you try to use every integer rank? from, to, index
and i never take values other than [0..9], right? So unsigned int
(or u8 if you somehow think that would generate better code) should be
good for them all?
> + /* Parse %pl[From[To]] */
> + for (i = 0; i < STRING_UNITS_2_NUM; i++)
> + if (fmt[1] == string_units_2[i][0])
> + break;
> + if (i < STRING_UNITS_2_NUM) {
> + from = i;
> + for (i = 0; i < STRING_UNITS_2_NUM; i++)
> + if (fmt[2] == string_units_2[i][0])
> + break;
> + if (i < STRING_UNITS_2_NUM && i >= from)
> + to = i;
> + }
>
> if (value)
> index = __ffs64(value) / 10;
> else
> index = 0;
> + index = clamp_t(unsigned long, index, from, to);
And then you wouldn't need to use the _t version here.
> /* Print numeric part */
> buf = number(buf, end, value >> (index * 10), default_dec_spec);
> @@ -1459,7 +1475,7 @@ int kptr_restrict __read_mostly;
> * Do not use this feature without some mechanism to verify the
> * correctness of the format string and va_list arguments.
> * - 'K' For a kernel pointer that should be hidden from unprivileged users
> - * - 'l' For a value in human-readable form (with IEC binary prefix)
> + * - 'l[from[to]]' For a value in human-readable form (with IEC binary prefix)
> * - 'NF' For a netdev_features_t
> * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
> * a certain separator (' ' by default):
Powered by blists - more mailing lists