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 <[email protected]>
> ---
> 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)
