On Thursday 09 April 2009 11:32:58 Kamil Dudka wrote:
> On Thursday 09 April 2009 09:23:37 Sven Joachim wrote:
> > Note that the #.b# file is listed at the top in (1) and at the bottom in
> > (2), despite all filenames in the directory being the same!
>
> Thanks for discovering this! The transitive axiom of the predicate is
> broken. I am working on a fix. It seems like a regression from this commit:
> http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=0443c2f3994
>3017f0aaa0afacbf68fb725858963

The fix for hidden files was based on a really bad idea. I've replaced it
with a special handle for "", "." and "..".

Each strcmp compares max 2 or 3 chars, so there is no performance impact 
(tested with -02 with zero impact; with -O0 it takes about 2% more time on 
sorting 100000 items).

The behavior on the current test suite is unchanged and the suite has been 
enlarged with "" and "#.b#". Note that the suite has been already checking 
transitivity of the sort predicate, but the "#.b#" was not included.


Kamil
From 691393ce3b85773f574c78caa931b0c2b566709d Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdu...@redhat.com>
Date: Thu, 9 Apr 2009 13:22:23 +0200
Subject: [PATCH] filevercmp: fix regression

---
 ChangeLog               |    7 +++++++
 lib/filevercmp.c        |   21 +++++++++++++--------
 tests/test-filevercmp.c |    2 ++
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e4c9bf6..0e45e71 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2009-04-09  Kamil Dudka  <kdu...@redhat.com>
+
+	Fix regression in 'filevercmp' module. Thanks Sven Joachim
+	for reporting it.
+	* lib/filevercmp.c: Special handle for "", "." and "..".
+	* tests/test-filevercmp.c: Enlarge the set suite.
+
 2009-04-07  Jim Meyering  <meyer...@redhat.com>
 
 	useless-if-before-free: show how to remove braced useless free, too
diff --git a/lib/filevercmp.c b/lib/filevercmp.c
index 99c07db..caa4891 100644
--- a/lib/filevercmp.c
+++ b/lib/filevercmp.c
@@ -135,14 +135,19 @@ filevercmp (const char *s1, const char *s2)
   if (simple_cmp == 0)
     return 0;
 
-  /* handle hidden files */
-  while (*s1 == '.' || *s2 == '.')
-    {
-      if (*s1 != *s2)
-	return *s1 - *s2;
-      s1++;
-      s2++;
-    }
+  /* special handle for "", "." and ".." */
+  if (!*s1)
+    return -1;
+  if (!*s2)
+    return 1;
+  if (0 == strcmp (".", s1))
+    return -1;
+  if (0 == strcmp (".", s2))
+    return 1;
+  if (0 == strcmp ("..", s1))
+    return -1;
+  if (0 == strcmp ("..", s2))
+    return 1;
 
   /* "cut" file suffixes */
   s1_pos = s1;
diff --git a/tests/test-filevercmp.c b/tests/test-filevercmp.c
index 9933b8a..700e182 100644
--- a/tests/test-filevercmp.c
+++ b/tests/test-filevercmp.c
@@ -37,6 +37,7 @@
 /* set of well sorted examples */
 static const char *const examples[] =
 {
+  "",
   ".",
   "..",
   ".a~",
@@ -73,6 +74,7 @@ static const char *const examples[] =
   "nss_ldap-1.0-0.1a.tar.gz",
   "nss_ldap-10beta1.fc8.tar.gz",
   "nss_ldap-10.11.8.6.20040204cvs.fc10.ebuild",
+  "#.b#",
   NULL
 };
 
-- 
1.6.1.2

Reply via email to