linux-kernel - Re: [PATCH 05/14] lib/vsprintf.c: help gcc make number() smaller

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHp75Vf2mhx_LmoOyEt04EMTGRB+BjGFtqJEmb91j32z7TT00Q@mail.gmail.com>
Date:	Tue, 24 Nov 2015 00:17:57 +0200
From:	Andy Shevchenko <andy.shevchenko@...il.com>
To:	Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 05/14] lib/vsprintf.c: help gcc make number() smaller

On Mon, Nov 23, 2015 at 11:29 PM, Rasmus Villemoes
<linux@...musvillemoes.dk> wrote:
> One consequence of the reorganization of struct printf_spec to make
> field_width 24 bits was that number() gained about 180 bytes. Since
> spec is never passed to other functions, we can help gcc make number()
> lose most of that extra weight by using local variables for the field
> width and precision.
>

Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>

One minor nitpick below

> Signed-off-by: Rasmus Villemoes <linux@...musvillemoes.dk>
> ---
>  lib/vsprintf.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 01c3aa638582..d7e27c54fa00 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -399,6 +399,8 @@ char *number(char *buf, char *end, unsigned long long num,
>         int need_pfx = ((spec.flags & SPECIAL) && spec.base != 10);
>         int i;
>         bool is_zero = num == 0LL;
> +       int field_width = spec.field_width;
> +       int precision = spec.precision;

I would put these before int i; But your choice.

>
>         /* locase = 0 or 0x20. ORing digits or letters with 'locase'
>          * produces same digits or (maybe lowercased) letters */
> @@ -410,20 +412,20 @@ char *number(char *buf, char *end, unsigned long long num,
>                 if ((signed long long)num < 0) {
>                         sign = '-';
>                         num = -(signed long long)num;
> -                       spec.field_width--;
> +                       field_width--;
>                 } else if (spec.flags & PLUS) {
>                         sign = '+';
> -                       spec.field_width--;
> +                       field_width--;
>                 } else if (spec.flags & SPACE) {
>                         sign = ' ';
> -                       spec.field_width--;
> +                       field_width--;
>                 }
>         }
>         if (need_pfx) {
>                 if (spec.base == 16)
> -                       spec.field_width -= 2;
> +                       field_width -= 2;
>                 else if (!is_zero)
> -                       spec.field_width--;
> +                       field_width--;
>         }
>
>         /* generate full string in tmp[], in reverse order */
> @@ -445,12 +447,12 @@ char *number(char *buf, char *end, unsigned long long num,
>         }
>
>         /* printing 100 using %2d gives "100", not "00" */
> -       if (i > spec.precision)
> -               spec.precision = i;
> +       if (i > precision)
> +               precision = i;
>         /* leading space padding */
> -       spec.field_width -= spec.precision;
> +       field_width -= precision;
>         if (!(spec.flags & (ZEROPAD | LEFT))) {
> -               while (--spec.field_width >= 0) {
> +               while (--field_width >= 0) {
>                         if (buf < end)
>                                 *buf = ' ';
>                         ++buf;
> @@ -479,14 +481,14 @@ char *number(char *buf, char *end, unsigned long long num,
>         if (!(spec.flags & LEFT)) {
>                 char c = ' ' + (spec.flags & ZEROPAD);
>                 BUILD_BUG_ON(' ' + ZEROPAD != '0');
> -               while (--spec.field_width >= 0) {
> +               while (--field_width >= 0) {
>                         if (buf < end)
>                                 *buf = c;
>                         ++buf;
>                 }
>         }
>         /* hmm even more zero padding? */
> -       while (i <= --spec.precision) {
> +       while (i <= --precision) {
>                 if (buf < end)
>                         *buf = '0';
>                 ++buf;
> @@ -498,7 +500,7 @@ char *number(char *buf, char *end, unsigned long long num,
>                 ++buf;
>         }
>         /* trailing space padding */
> -       while (--spec.field_width >= 0) {
> +       while (--field_width >= 0) {
>                 if (buf < end)
>                         *buf = ' ';
>                 ++buf;
> --
> 2.6.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/