Message-ID: <CAMuHMdUp3fOkAttk2FKJTh5svaiMwgu4JZ8utPeHghU4TQLE-w@mail.gmail.com>
Date: Fri, 26 Apr 2024 17:27:08 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Rasmus Villemoes <linux@...musvillemoes.dk>
Cc: Matthew Wilcox <willy@...radead.org>, Petr Mladek <pmladek@...e.com>, 
	Steven Rostedt <rostedt@...dmis.org>, Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, 
	Sergey Senozhatsky <senozhatsky@...omium.org>, Randy Dunlap <rdunlap@...radead.org>, 
	linux-kernel@...r.kernel.org
Subject: Re: [RFC] Printing numbers in SI units

Hi Rasmus,

On Wed, Jan 24, 2024 at 11:43 PM Rasmus Villemoes
<linux@...musvillemoes.dk> wrote:
> On 24/01/2024 19.58, Matthew Wilcox wrote:
> > I was looking at hugetlbfs and it has several snippets of code like
> > this:
> >
> >         string_get_size(huge_page_size(h), 1, STRING_UNITS_2, buf, 32);
> >         pr_warn("HugeTLB: allocating %u of page size %s failed node%d.  Only allocated %lu hugepages.\n",
> >                 h->max_huge_pages_node[nid], buf, nid, i);
> >
> > That's not terribly ergonomic, so I wondered if I could do better.
> > Unfortunately, I decided to do it using the SPECIAL flag which GCC
> > warns about.  But I've written the code now, so I'm sending it out in
> > case anybody has a better idea for how to incorporate it.
>
> Well, something that gcc will warn about with Wformat isn't gonna fly,
> obviously. But my man page also mentions ' as a possible flag for d
> conversions:
>
>        '      For decimal conversion (i, d, u, f, F, g, G) the output is
> to be grouped with thousands'
>               grouping characters if the locale information indicates any.

> Obviously, our printf wouldn't implement that, [...]

Why not? ;-)

Old Gmail-white-space-damaged patch below, which I wrote when I got
fed up with meticulously counting zeros in GHz-range clock
frequencies...

Author: Geert Uytterhoeven <geert+renesas@...der.be>
Date:   Thu Aug 11 13:52:46 2016 +0200

    lib/vsprintf.c: Add support for thousands' grouping

    Use an underscore as the grouping character.

    TODO:
      - Documentation
      - Self test
      - Do we want to use this in /sys/kernel/debug/clk/clk_summary ?
        RFC patch, compatibility was already broken by commit
        e55a839a7a1c561b ("clk: add clock protection mechanism to clk
        core")

    Signed-off-by: Geert Uytterhoeven <geert+renesas@...der.be>
    ---
    Originally I wanted to use grouping by 4 for octal and hexadecimal
    numbers.  Unfortunately gcc prints a warning when using the grouping
    character with octal and hexadecimal numbers:

        warning: ''' flag used with '%x' gnu_printf format [-Wformat=]
        warning: ''' flag used with '%o' gnu_printf format [-Wformat=]

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index f4a8bfacb70befbc..51e516cf4cc68156 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -414,6 +414,9 @@ int num_to_str(char *buf, int size, unsigned long
long num, unsigned int width)
 #define ZEROPAD        16              /* pad with zero, must be 16
== '0' - ' ' */
 #define SMALL  32              /* use lowercase in hex (must be 32 == 0x20) */
 #define SPECIAL        64              /* prefix hex with "0x", octal
with "0" */
+#define GROUP  128             /* thousands' grouping */
+// warning: ''' flag used with '%x' gnu_printf format [-Wformat=]
+// warning: ''' flag used with '%o' gnu_printf format [-Wformat=]

 static_assert(SIGN == 1);
 static_assert(ZEROPAD == ('0' - ' '));
@@ -462,7 +465,7 @@ char *number(char *buf, char *end, unsigned long long num,
        char sign;
        char locase;
        int need_pfx = ((spec.flags & SPECIAL) && spec.base != 10);
-       int i;
+       int i, j;
        bool is_zero = num == 0LL;
        int field_width = spec.field_width;
        int precision = spec.precision;
@@ -511,6 +514,11 @@ char *number(char *buf, char *end, unsigned long long num,
                i = put_dec(tmp, num) - tmp;
        }

+       /* take into account grouping characters */
+       j = i;
+       if (spec.flags & GROUP)
+               i += (j - 1) / 3;
+
        /* printing 100 using %2d gives "100", not "00" */
        if (i > precision)
                precision = i;
@@ -559,10 +567,16 @@ char *number(char *buf, char *end, unsigned long long num,
                ++buf;
        }
        /* actual digits of result */
-       while (--i >= 0) {
+       while (--j >= 0) {
                if (buf < end)
-                       *buf = tmp[i];
+                       *buf = tmp[j];
                ++buf;
+               if ((spec.flags & GROUP) && j && !(j % 3)) {
+                       if (buf < end)
+                               // FIXME '\''
+                               *buf = '_';
+                       ++buf;
+               }
        }
        /* trailing space padding */
        while (--field_width >= 0) {
@@ -2567,6 +2581,7 @@ int format_decode(const char *fmt, struct
printf_spec *spec)
                case ' ': spec->flags |= SPACE;   break;
                case '#': spec->flags |= SPECIAL; break;
                case '0': spec->flags |= ZEROPAD; break;
+               case '\'': spec->flags |= GROUP;  break;
                default:  found = false;
                }

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68korg

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Powered by blists - more mailing lists