linux-kernel - Re: [PATCH]: improved strnicmp in lib/string.c

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: <20080507195317.GA5131@martell.zuzino.mipt.ru>
Date:	Wed, 7 May 2008 23:53:17 +0400
From:	Alexey Dobriyan <adobriyan@...il.com>
To:	Soumyadip Das Mahapatra <dip_kernel@...oo.co.in>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH]: improved strnicmp in lib/string.c

On Wed, May 07, 2008 at 11:51:09PM +0530, Soumyadip Das Mahapatra wrote:
> This is somewhat improved version of strnicmp() function in lib/string.c.

Familiarize youself with coding style before sending patches, see
kernel/sched.c , fs/dcache.c for good examples.

>I have implemented binary
> comparison  rather than linear one(which was in the older version).

Obviously buggy.

> --- 2.6.25-vanilla/lib/string.c
> +++ 2.6.25-hacked/lib/string.c
> @@ -31,32 +10,43 @@
>   * @s2: The other string
>   * @len: the maximum number of characters to compare
>   */
> +/* This is somewhat faster
> + * method compared to the
> + * olderone (in case of a large string)
> + */
>  int strnicmp(const char *s1, const char *s2, size_t len)
>  {
> -    /* Yes, Virginia, it had better be unsigned */
> -    unsigned char c1, c2;
> -
> -    c1 = c2 = 0;
> -    if (len) {
> -        do {
> -            c1 = *s1;
> -            c2 = *s2;
> -            s1++;
> -            s2++;
> -            if (!c1)
> -                break;
> -            if (!c2)
> -                break;
> -            if (c1 == c2)
> -                continue;
> -            c1 = tolower(c1);
> -            c2 = tolower(c2);
> -            if (c1 != c2)
> +    /* Yes, i am keeping 'em unsigned too */
> +    unsigned char c1, c2, c3, c4;
> +    unsigned count = 0;
> +    int flag = -1;
> +
> +    c1 = c2 = c3 = c4 = 0;
> +
> +    if(len > 0)
> +    {
> +        for(; count <= len/2; count++)
> +        {
> +            c1 = tolower(s1[count]);    // well, we
> +            c2 = tolower(s2[count]);    // are ignoring
> +            c3 = tolower(s1[len-count-1]);    // cases
> +            c4 = tolower(s2[len-count-1]);    // thats why

Buggy.

> +            
> +            if(c1 == c2)
> +            {
> +                if(c3 != c4)
> +                {
> +                    flag = 1;
> +                    break;
> +                }
> +            }
> +            else
>                  break;
> -        } while (--len);
> +            flag = 0;
> +        }
>      }
> -    return (int)c1 - (int)c2;
> -}
> +    return flag;     // return 0 for matching and 
> +}            // nonzero for mismatch
>  EXPORT_SYMBOL(strnicmp);
>  #endif

And this function is called at most ~100 character strings, so nobody
cares about its performance.

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