bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] (x)memcoll: minor tweaks


From: Paul Eggert
Subject: [PATCH] (x)memcoll: minor tweaks
Date: Thu, 08 Jul 2010 18:33:17 -0700
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.10) Gecko/20100527 Thunderbird/3.0.5

I looked through that patch and installed the following relatively minor tweaks.
The only nontrivial one is that I removed the abort() checks from memcoll0.
Performance does matter with 'sort', and I think it unlikely that the abort()
checks would catch important errors that relatively simple testing would not 
catch.
(One bit of evidence to support this opinion is that the abort() checks
themselves were incorrect.)

>From 277aeb362143c763a4297d67c3ce55923649a6bd Mon Sep 17 00:00:00 2001
From: Paul Eggert <address@hidden>
Date: Thu, 8 Jul 2010 18:16:40 -0700
Subject: [PATCH] (x)memcoll: minor tweaks

* lib/memcoll.c (strcoll_loop): Prefer the style where 'const'
is after the type that it qualifies.
(memcoll0): Likewise.
* lib/memcoll.h (memcoll0): Likewise.
* lib/xmemcoll.c (collate_error, xmemcoll0): Likewise.
* lib/xmemcoll.h (xmemcoll0): Likewise.
* lib/memcoll.c (memcoll0): Correct the comment.  This function
differs from memcoll in that the NUL byte is part of the argument.
Omit the abort-checks, as performance is a real issue here.  Plus,
the checks were wrong anyway (an off-by-one error).  Omit local
variable 'diff', as it's a bit clearer that way.
* m4/memcoll.m4 (gl_MEMCOLL): Omit AC_FUNC_STRCOLL, as it's
no longer needed.
---
 ChangeLog      |   17 +++++++++++++++++
 lib/memcoll.c  |   25 ++++++++++---------------
 lib/memcoll.h  |    2 +-
 lib/xmemcoll.c |   12 +++++-------
 lib/xmemcoll.h |    2 +-
 m4/memcoll.m4  |    5 +----
 6 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 1577e12..1207bef 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2010-07-08  Paul Eggert  <address@hidden>
+
+       (x)memcoll: minor tweaks
+       * lib/memcoll.c (strcoll_loop): Prefer the style where 'const'
+       is after the type that it qualifies.
+       (memcoll0): Likewise.
+       * lib/memcoll.h (memcoll0): Likewise.
+       * lib/xmemcoll.c (collate_error, xmemcoll0): Likewise.
+       * lib/xmemcoll.h (xmemcoll0): Likewise.
+       * lib/memcoll.c (memcoll0): Correct the comment.  This function
+       differs from memcoll in that the NUL byte is part of the argument.
+       Omit the abort-checks, as performance is a real issue here.  Plus,
+       the checks were wrong anyway (an off-by-one error).  Omit local
+       variable 'diff', as it's a bit clearer that way.
+       * m4/memcoll.m4 (gl_MEMCOLL): Omit AC_FUNC_STRCOLL, as it's
+       no longer needed.
+
 2010-07-08  Chen Guo <address@hidden>
 
        (x)memcoll: speedup when input is known to be NUL delimited
diff --git a/lib/memcoll.c b/lib/memcoll.c
index 8e48551..580f81d 100644
--- a/lib/memcoll.c
+++ b/lib/memcoll.c
@@ -30,9 +30,10 @@
    NUL bytes. */
 
 static inline int
-strcoll_loop (const char *s1, size_t s1len, const char *s2, size_t s2len)
+strcoll_loop (char const *s1, size_t s1len, const char *s2, size_t s2len)
 {
   int diff;
+
   while (! (errno = 0, (diff = strcoll (s1, s2)) || errno))
     {
       /* strcoll found no difference, but perhaps it was fooled by NUL
@@ -57,6 +58,7 @@ strcoll_loop (const char *s1, size_t s1len, const char *s2, 
size_t s2len)
           break;
         }
     }
+
   return diff;
 }
 
@@ -65,7 +67,6 @@ strcoll_loop (const char *s1, size_t s1len, const char *s2, 
size_t s2len)
    adjacent.  Perhaps temporarily modify the bytes after S1 and S2,
    but restore their original contents before returning.  Set errno to an
    error number if there is an error, and to zero otherwise.  */
-
 int
 memcoll (char *s1, size_t s1len, char *s2, size_t s2len)
 {
@@ -97,24 +98,18 @@ memcoll (char *s1, size_t s1len, char *s2, size_t s2len)
   return diff;
 }
 
-/* Like memcoll, but S1 and S2 are known to be NUL delimited, thus no
-   modification to S1 or S2 are needed. */
+/* Compare S1 (with length S1LEN) and S2 (with length S2LEN) according
+   to the LC_COLLATE locale.  S1 and S2 must both end in a null byte.
+   Set errno to an error number if there is an error, and to zero
+   otherwise.  */
 int
-memcoll0 (const char *s1, size_t s1len, const char *s2, size_t s2len)
+memcoll0 (char const *s1, size_t s1len, const char *s2, size_t s2len)
 {
-  int diff;
-  if (!(s1len > 0 && s1[s1len] == '\0'))
-    abort ();
-  if (!(s2len > 0 && s2[s2len] == '\0'))
-    abort ();
-
   if (s1len == s2len && memcmp (s1, s2, s1len) == 0)
     {
       errno = 0;
-      diff = 0;
+      return 0;
     }
   else
-    diff = strcoll_loop (s1, s1len, s2, s2len);
-
-  return diff;
+    return strcoll_loop (s1, s1len, s2, s2len);
 }
diff --git a/lib/memcoll.h b/lib/memcoll.h
index b43a96e..1e8ce48 100644
--- a/lib/memcoll.h
+++ b/lib/memcoll.h
@@ -23,6 +23,6 @@
 # include <stddef.h>
 
 int memcoll (char *, size_t, char *, size_t);
-int memcoll0 (const char *, size_t, const char *, size_t);
+int memcoll0 (char const *, size_t, char const *, size_t);
 
 #endif /* MEMCOLL_H_ */
diff --git a/lib/xmemcoll.c b/lib/xmemcoll.c
index 36958f4..7f8c894 100644
--- a/lib/xmemcoll.c
+++ b/lib/xmemcoll.c
@@ -31,9 +31,10 @@
 #include "quotearg.h"
 #include "xmemcoll.h"
 
-static inline void
-collate_error (int collation_errno, const char *s1, size_t s1len,
-               const char *s2, size_t s2len)
+static void
+collate_error (int collation_errno,
+               char const *s1, size_t s1len,
+               char const *s2, size_t s2len)
 {
   error (0, collation_errno, _("string comparison failed"));
   error (0, 0, _("Set LC_ALL='C' to work around the problem."));
@@ -54,10 +55,8 @@ xmemcoll (char *s1, size_t s1len, char *s2, size_t s2len)
 {
   int diff = memcoll (s1, s1len, s2, s2len);
   int collation_errno = errno;
-
   if (collation_errno)
     collate_error (collation_errno, s1, s1len, s2, s2len);
-
   return diff;
 }
 
@@ -65,11 +64,10 @@ xmemcoll (char *s1, size_t s1len, char *s2, size_t s2len)
    no modifications to S1 and S2 are needed. */
 
 int
-xmemcoll0 (const char *s1, size_t s1len, const char *s2, size_t s2len)
+xmemcoll0 (char const *s1, size_t s1len, char const *s2, size_t s2len)
 {
   int diff = memcoll0 (s1, s1len, s2, s2len);
   int collation_errno = errno;
-
   if (collation_errno)
     collate_error (collation_errno, s1, s1len, s2, s2len);
   return diff;
diff --git a/lib/xmemcoll.h b/lib/xmemcoll.h
index b72127d..4170b9b 100644
--- a/lib/xmemcoll.h
+++ b/lib/xmemcoll.h
@@ -1,3 +1,3 @@
 #include <stddef.h>
 int xmemcoll (char *, size_t, char *, size_t);
-int xmemcoll0 (const char *, size_t, const char *, size_t);
+int xmemcoll0 (char const *, size_t, char const *, size_t);
diff --git a/m4/memcoll.m4 b/m4/memcoll.m4
index 0edf30c..220b9d3 100644
--- a/m4/memcoll.m4
+++ b/m4/memcoll.m4
@@ -1,4 +1,4 @@
-# memcoll.m4 serial 7
+# memcoll.m4 serial 8
 dnl Copyright (C) 2002-2003, 2005-2006, 2009-2010 Free Software Foundation,
 dnl Inc.
 dnl This file is free software; the Free Software Foundation
@@ -9,7 +9,4 @@ AC_DEFUN([gl_MEMCOLL],
 [
   AC_REQUIRE([AC_C_INLINE])
   AC_LIBOBJ([memcoll])
-
-  dnl Prerequisites of lib/memcoll.c.
-  AC_FUNC_STRCOLL
 ])
-- 
1.7.0.4




reply via email to

[Prev in Thread] Current Thread [Next in Thread]