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