On 11/05/13 at 11:03pm, Dave Reisner wrote:
This refactors the existing vercmp machinery in order to:
1) Correct a deficiency in files comparison. This was too lazily written and could produce incorrect results when a version contained an epoch. 2) Improve tarball detection. Calling fnmatch a second time for the uncompressed tarball case is not a noticeable performance hit, and is more correct. 3) Fix a bug in tarball detection. The glob pattern would match files with only a pkgver, e.g. foopkg-1.2.3-x86_64.pkg.tar.xz. 4) Be generally cleaner code, easier to follow.
The test suite gets extended to validate my claims for the new behavior.
Fixes FS#37631.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- This supersedes an earlier patch I sent against pacsort which was a bit more directed and less useful.
src/util/pacsort.c | 164 ++++++++++++++++++++++++++++++++++------------- test/util/pacsorttest.sh | 31 ++++++++- 2 files changed, 150 insertions(+), 45 deletions(-)
diff --git a/src/util/pacsort.c b/src/util/pacsort.c index 687e558..554352c 100644 --- a/src/util/pacsort.c +++ b/src/util/pacsort.c
<snip>
+ const char *pkgver_end; + const char *slash; + + /* Find the basename */ + slash = strrchr(path, '/'); + meta->pkgname = slash ? slash + 1 : path; + pkgver_end = strrchr(meta->pkgname, '-'); + + /* Read backwards through pkgrel */ + for(meta->pkgver = pkgver_end - 1; + meta->pkgver > meta->pkgname && *meta->pkgver != '-'; + --meta->pkgver) + ; + /* Read backwards through pkgver */ + for(--meta->pkgver; + meta->pkgver > meta->pkgname && *meta->pkgver != '-'; + --meta->pkgver) + ;
Nitpick. Personally, I would have an easier time reading this if we used a temporary variable for meta->pkgver so that the for loop could fit all on one line the way it is in _alpm_splitname.
+ ++meta->pkgver; + + meta->pkgname_len = meta->pkgver - meta->pkgname - 1; + meta->pkgver_len = pkgver_end - meta->pkgver; +}
<snip>
+static int vercmp_files(const char *file1, const char *file2) +{ + struct pkgmeta_t pkg1, pkg2; + + /* In order for 2 files to be considered equal, they must have the same + * basename. For example, the two follow strings are considered equal: + * /var/cache/pacman/pkg/pacman-4.1.1-1-x86_64.pkg.tar.gz + * /share/packages/pacman-4.1.1-1-x86_64.pkg.tar.gz + */ + + extract_pkgmeta(file1, &pkg1); + extract_pkgmeta(file2, &pkg2);
We could do this extraction before the sort instead of for every comparison so that we sort a *pkg_meta_t[] instead of *char[]. Then we could also verify that our input consists entirely of paths, because mixed paths and version numbers with --files will give nonsensical results that change based on the order of the input.
+ + return pkgmeta_cmp(&pkg1, &pkg2); +} + +static int vercmp(const void *v1, const void *v2) +{ + const char *s1 = *(const char **)v1, *s2 = *(const char **)v2; + int cmp; + + if(opts.filemode && is_pkgtar(s1) && is_pkgtar(s1)) {
You check s1 twice.
+ cmp = vercmp_files(s1, s2); + } else { + cmp = vercmp_versions(s1, s2); }
- return r; + return opts.order * cmp; }
static char escape_char(const char *string)