[PATCH]: improved strnicmp in lib/string.c

[PATCH]: improved strnicmp in lib/string.c

[Soumyadip Das Mahapatra]

This is somewhat improved version of strnicmp() function in lib/string.c. I have implemented binary
comparison  rather than linear one(which was in the older version). I am appending the patch bellow

--- 2.6.25-vanilla/lib/string.c    2008-04-17 08:19:44.000000000 +0530
+++ 2.6.25-hacked/lib/string.c    2008-05-07 23:31:57.000000000 +0530
@@ -1,24 +1,3 @@
-/*
- *  linux/lib/string.c
- *
- *  Copyright (C) 1991, 1992  Linus Torvalds
- */
-
-/*
- * stupid library routines.. The optimized versions should generally be found
- * as inline code in <asm-xx/string.h>
- *
- * These are buggy as well..
- *
- * * Fri Jun 25 1999, Ingo Oeser <ioe@informatik.tu-chemnitz.de>
- * -  Added strsep() which will replace strtok() soon (because strsep() is
- *    reentrant and should be faster). Use only strsep() in new code, please.
- *
- * * Sat Feb 09 2002, Jason Thomas <jason@topic.com.au>,
- *                    Matthew Hawkins <matt@mh.dropbear.id.au>
- * -  Kissed strtok() goodbye
- */
-
 #include <linux/types.h>
 #include <linux/string.h>
 #include <linux/ctype.h>
@@ -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
+            
+            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



      Meet people who discuss and share your passions. Go to http://in.promos.yahoo.com/groups/bestofyahoo/

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

[Pekka Enberg]

On Wed, May 7, 2008 at 9:21 PM, Soumyadip Das Mahapatra
<dip_kernel@yahoo.co.in> wrote:
>   int strnicmp(const char *s1, const char *s2, size_t len)
>   {
>  +    /* 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
>  +
>  +            if(c1 == c2)
>  +            {
>  +                if(c3 != c4)
>  +                {
>  +                    flag = 1;
>  +                    break;
>  +                }
>  +            }
>  +            else
>                  break;
>  -        } while (--len);
>  +            flag = 0;

Unconditionally setting flag to zero in the loop also broken. For
example, comparing "hello" and "he9lo" resunts into zero now.

>  +        }
>      }
>  -    return (int)c1 - (int)c2;
>  -}
>  +    return flag;     // return 0 for matching and
>  +}            // nonzero for mismatch
>   EXPORT_SYMBOL(strnicmp);
>   #endif
>
>
>
>       Meet people who discuss and share your passions. Go to http://in.promos.yahoo.com/groups/bestofyahoo/
>
>  --
>  To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>  the body of a message to majordomo@vger.kernel.org
>  More majordomo info at  http://vger.kernel.org/majordomo-info.html
>  Please read the FAQ at  http://www.tux.org/lkml/
>

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

[Alexey Dobriyan]

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.

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

[David Newall]

Soumyadip Das Mahapatra wrote:
> This is somewhat improved version of strnicmp() function in lib/string.c. I have implemented binary
> comparison  rather than linear one(which was in the older version).

It doesn't correctly indicate the sort order.  For example, when
comparing "accdd" against "abcde", it indicates that the first string
sorts before the second (which is wrong.)