linux-kernel - Re: [PATCH 1/1] lib: small update for strlen, strnlen, use less cpu instructions

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]

Date:	Wed, 17 Jun 2015 22:07:27 +0200
From:	Richard Weinberger <richard@....at>
To:	Orestes Leal Rodriguez <lukes357@...il.com>, bp@...en8.de
CC:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] lib: small update for strlen, strnlen, use less cpu
 instructions

Am 17.06.2015 um 00:51 schrieb Orestes Leal Rodriguez:
>> Use the force^Wcheckpatch.pl.
> This is the output of checkpatch.pl:
> output of checkpatch: total: 0 errors, 0 warnings, 42 lines checked
> /root/string.c.patch has no obvious style problems and is ready for submission

But it does not apply at all.
Did you test it? I fear your mail client did some whitespace damage.

>> You need to explain that in the commit message, my young padawan.
> Very small update to strlen and strnlen that now use less cpu instructions by using a counter to avoid memory address
> arithmetic, which cause that the compiler adds more machine
> instructions for computing the length of the string just before
> returning from the functions, the old machine code is like the
> following:
> 
>  mov    -0x4(%ebp),%edx
>  mov    0x8(%ebp),%eax
>  sub    %eax,%edx
>  mov    %edx,%eax
>  leave
>  ret
> 
> 
> now in the new versions the value is not calculated anymore,
> instead  he value of the counter is put on eax after the
> condition inside the loop no longer holds, and then return:
> 
>  mov    -0x4(%ebp),%eax
>  leave
>  ret
> 
> With this a few cpu instructions are saved.

x86_32 does not matter here as we have already an optimized strlen() in
arch/x86/lib/string_32.c.
Did you check whether the optimization is worth on other archs?
Hint: grep __HAVE_ARCH_STRLEN

> 
> Signed-off-by: Orestes Leal Rodriguez <lukes357@...il.com>
> ---
> 
> Signed-off-by: Orestes Leal Rodriguez <lukes357@...il.com>

What does this 2nd SoB here?

> diff --git a/lib/string.c b/lib/string.c
> index 992bf30..c873436 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -17,6 +17,10 @@
>   * * Sat Feb 09 2002, Jason Thomas <jason@...ic.com.au>,
>   *                    Matthew Hawkins <matt@...dropbear.id.au>
>   * -  Kissed strtok() goodbye
> + *
> + * * Tuesday June 16 2015, Orestes Leal Rodriguez <lukes357@...il.com>
> + * - strlen, strnlen: by using a single counter we use less cpu instructions
> + *   by avoiding substracting the memory addresses before return

No need to add anything here. These days we have git. :-)

>   */
> 
>  #include <linux/types.h>
> @@ -401,11 +405,11 @@ EXPORT_SYMBOL(strim);
>   */
>  size_t strlen(const char *s)
>  {
> -    const char *sc;
> +    size_t sz = 0;
> 
> -    for (sc = s; *sc != '\0'; ++sc)
> -        /* nothing */;
> -    return sc - s;
> +    for (; *s++ != '\0'; sz++)
> +        /* empty */;

<nitpick>
Why suddenly "empty" instead of "nothing"?
</nitpick>

> +    return sz;
>  }
>  EXPORT_SYMBOL(strlen);
>  #endif
> @@ -418,12 +422,13 @@ EXPORT_SYMBOL(strlen);
>   */
>  size_t strnlen(const char *s, size_t count)
>  {
> -    const char *sc;
> +    size_t sz = 0;
> 
> -    for (sc = s; count-- && *sc != '\0'; ++sc)
> -        /* nothing */;
> -    return sc - s;
> +    for (; count-- && *s++ != '\0'; sz++)
> +        /* empty */;

Same here.

Thanks,
//R2D2
--
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/