[pacman-dev] [PATCH 1/2] lib/version: versions must be same length to be equal
vercmp exhibited some odd behavior here, declaring versions such as '23.3a' and '23.3.a' as equivalent. Add an extra check before we return equality that the versions have the same length. Adds a pair of test cases to the vercmp test suite as well. Reported-by: Eric Belanger <eric@archlinux.org> Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- lib/libalpm/version.c | 13 ++++++++++++- test/util/vercmptest.sh | 4 ++++ 2 files changed, 16 insertions(+), 1 deletions(-) diff --git a/lib/libalpm/version.c b/lib/libalpm/version.c index 9f3a9b7..bab9f92 100644 --- a/lib/libalpm/version.c +++ b/lib/libalpm/version.c @@ -88,6 +88,7 @@ static int rpmvercmp(const char *a, const char *b) char *str1, *str2; char *ptr1, *ptr2; char *one, *two; + size_t one_len, two_len; int rc; int isnum; int ret = 0; @@ -101,6 +102,9 @@ static int rpmvercmp(const char *a, const char *b) one = str1; two = str2; + one_len = strlen(one); + two_len = strlen(two); + /* loop through each version segment of str1 and str2 and compare them */ while(*one && *two) { while(*one && !isalnum((int)*one)) one++; @@ -187,9 +191,16 @@ static int rpmvercmp(const char *a, const char *b) /* this catches the case where all numeric and alpha segments have */ /* compared identically but the segment separating characters were */ - /* different */ + /* different. versions must also be the same length to be equivalent, */ + /* otherwise, the shorter version is declared newer. */ if ((!*one) && (!*two)) { + int diff = two_len - one_len; ret = 0; + if(diff > 0) { + ret = 1; + } else if (diff < 0) { + ret = -1; + } goto cleanup; } diff --git a/test/util/vercmptest.sh b/test/util/vercmptest.sh index 54ede04..ff498ba 100755 --- a/test/util/vercmptest.sh +++ b/test/util/vercmptest.sh @@ -137,6 +137,10 @@ runtest 1:1.0 1.0 1 runtest 1:1.0 1.1 1 runtest 1:1.1 1.1 1 +# same alnum content, different length +runtest 1a 1.a 1 +runtest 23.3.a 23.3a -1 + #END TESTS echo -- 1.7.6
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- lib/libalpm/version.c | 49 ++++++++++++++++++++++++++++++++----------------- 1 files changed, 32 insertions(+), 17 deletions(-) diff --git a/lib/libalpm/version.c b/lib/libalpm/version.c index bab9f92..f86e0c0 100644 --- a/lib/libalpm/version.c +++ b/lib/libalpm/version.c @@ -107,8 +107,12 @@ static int rpmvercmp(const char *a, const char *b) /* loop through each version segment of str1 and str2 and compare them */ while(*one && *two) { - while(*one && !isalnum((int)*one)) one++; - while(*two && !isalnum((int)*two)) two++; + while(*one && !isalnum((int)*one)) { + one++; + } + while(*two && !isalnum((int)*two)) { + two++; + } /* If we ran to the end of either, we are finished with the loop */ if(!(*one && *two)) break; @@ -120,12 +124,20 @@ static int rpmvercmp(const char *a, const char *b) /* leave one and two pointing to the start of the alpha or numeric */ /* segment and walk ptr1 and ptr2 to end of segment */ if(isdigit((int)*ptr1)) { - while(*ptr1 && isdigit((int)*ptr1)) ptr1++; - while(*ptr2 && isdigit((int)*ptr2)) ptr2++; + while(*ptr1 && isdigit((int)*ptr1)) { + ptr1++; + } + while(*ptr2 && isdigit((int)*ptr2)) { + ptr2++; + } isnum = 1; } else { - while(*ptr1 && isalpha((int)*ptr1)) ptr1++; - while(*ptr2 && isalpha((int)*ptr2)) ptr2++; + while(*ptr1 && isalpha((int)*ptr1)) { + ptr1++; + } + while(*ptr2 && isalpha((int)*ptr2)) { + ptr2++; + } isnum = 0; } @@ -138,7 +150,7 @@ static int rpmvercmp(const char *a, const char *b) /* this cannot happen, as we previously tested to make sure that */ /* the first string has a non-null segment */ - if (one == ptr1) { + if(one == ptr1) { ret = -1; /* arbitrary */ goto cleanup; } @@ -147,26 +159,30 @@ static int rpmvercmp(const char *a, const char *b) /* different types: one numeric, the other alpha (i.e. empty) */ /* numeric segments are always newer than alpha segments */ /* XXX See patch #60884 (and details) from bugzilla #50977. */ - if (two == ptr2) { + if(two == ptr2) { ret = isnum ? 1 : -1; goto cleanup; } - if (isnum) { + if(isnum) { /* this used to be done by converting the digit segments */ /* to ints using atoi() - it's changed because long */ /* digit segments can overflow an int - this should fix that. */ /* throw away any leading zeros - it's a number, right? */ - while (*one == '0') one++; - while (*two == '0') two++; + while(*one == '0') { + one++; + } + while(*two == '0') { + two++; + } /* whichever number has more digits wins */ - if (strlen(one) > strlen(two)) { + if(strlen(one) > strlen(two)) { ret = 1; goto cleanup; } - if (strlen(two) > strlen(one)) { + if(strlen(two) > strlen(one)) { ret = -1; goto cleanup; } @@ -177,7 +193,7 @@ static int rpmvercmp(const char *a, const char *b) /* if they are equal because there might be more segments to */ /* compare */ rc = strcmp(one, two); - if (rc) { + if(rc) { ret = rc < 1 ? -1 : 1; goto cleanup; } @@ -193,7 +209,7 @@ static int rpmvercmp(const char *a, const char *b) /* compared identically but the segment separating characters were */ /* different. versions must also be the same length to be equivalent, */ /* otherwise, the shorter version is declared newer. */ - if ((!*one) && (!*two)) { + if((!*one) && (!*two)) { int diff = two_len - one_len; ret = 0; if(diff > 0) { @@ -210,8 +226,7 @@ static int rpmvercmp(const char *a, const char *b) * - if one is an alpha, two is newer. * - otherwise one is newer. * */ - if ( (!*one && !isalpha((int)*two)) - || isalpha((int)*one) ) { + if((!*one && !isalpha((int)*two)) || isalpha((int)*one) ) { ret = -1; } else { ret = 1; -- 1.7.6
On Sat, Aug 20, 2011 at 9:42 PM, Dave Reisner <d@falconindy.com> wrote:
Signed-off-by: Dave Reisner <dreisner@archlinux.org> NACK. This makes diffing with the RPM code a lot tougher. Read the comments at the top of the file- I thought I noted this?
--- lib/libalpm/version.c | 49 ++++++++++++++++++++++++++++++++----------------- 1 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/lib/libalpm/version.c b/lib/libalpm/version.c index bab9f92..f86e0c0 100644 --- a/lib/libalpm/version.c +++ b/lib/libalpm/version.c @@ -107,8 +107,12 @@ static int rpmvercmp(const char *a, const char *b)
/* loop through each version segment of str1 and str2 and compare them */ while(*one && *two) { - while(*one && !isalnum((int)*one)) one++; - while(*two && !isalnum((int)*two)) two++; + while(*one && !isalnum((int)*one)) { + one++; + } + while(*two && !isalnum((int)*two)) { + two++; + }
/* If we ran to the end of either, we are finished with the loop */ if(!(*one && *two)) break; @@ -120,12 +124,20 @@ static int rpmvercmp(const char *a, const char *b) /* leave one and two pointing to the start of the alpha or numeric */ /* segment and walk ptr1 and ptr2 to end of segment */ if(isdigit((int)*ptr1)) { - while(*ptr1 && isdigit((int)*ptr1)) ptr1++; - while(*ptr2 && isdigit((int)*ptr2)) ptr2++; + while(*ptr1 && isdigit((int)*ptr1)) { + ptr1++; + } + while(*ptr2 && isdigit((int)*ptr2)) { + ptr2++; + } isnum = 1; } else { - while(*ptr1 && isalpha((int)*ptr1)) ptr1++; - while(*ptr2 && isalpha((int)*ptr2)) ptr2++; + while(*ptr1 && isalpha((int)*ptr1)) { + ptr1++; + } + while(*ptr2 && isalpha((int)*ptr2)) { + ptr2++; + } isnum = 0; }
@@ -138,7 +150,7 @@ static int rpmvercmp(const char *a, const char *b)
/* this cannot happen, as we previously tested to make sure that */ /* the first string has a non-null segment */ - if (one == ptr1) { + if(one == ptr1) { ret = -1; /* arbitrary */ goto cleanup; } @@ -147,26 +159,30 @@ static int rpmvercmp(const char *a, const char *b) /* different types: one numeric, the other alpha (i.e. empty) */ /* numeric segments are always newer than alpha segments */ /* XXX See patch #60884 (and details) from bugzilla #50977. */ - if (two == ptr2) { + if(two == ptr2) { ret = isnum ? 1 : -1; goto cleanup; }
- if (isnum) { + if(isnum) { /* this used to be done by converting the digit segments */ /* to ints using atoi() - it's changed because long */ /* digit segments can overflow an int - this should fix that. */
/* throw away any leading zeros - it's a number, right? */ - while (*one == '0') one++; - while (*two == '0') two++; + while(*one == '0') { + one++; + } + while(*two == '0') { + two++; + }
/* whichever number has more digits wins */ - if (strlen(one) > strlen(two)) { + if(strlen(one) > strlen(two)) { ret = 1; goto cleanup; } - if (strlen(two) > strlen(one)) { + if(strlen(two) > strlen(one)) { ret = -1; goto cleanup; } @@ -177,7 +193,7 @@ static int rpmvercmp(const char *a, const char *b) /* if they are equal because there might be more segments to */ /* compare */ rc = strcmp(one, two); - if (rc) { + if(rc) { ret = rc < 1 ? -1 : 1; goto cleanup; } @@ -193,7 +209,7 @@ static int rpmvercmp(const char *a, const char *b) /* compared identically but the segment separating characters were */ /* different. versions must also be the same length to be equivalent, */ /* otherwise, the shorter version is declared newer. */ - if ((!*one) && (!*two)) { + if((!*one) && (!*two)) { int diff = two_len - one_len; ret = 0; if(diff > 0) { @@ -210,8 +226,7 @@ static int rpmvercmp(const char *a, const char *b) * - if one is an alpha, two is newer. * - otherwise one is newer. * */ - if ( (!*one && !isalpha((int)*two)) - || isalpha((int)*one) ) { + if((!*one && !isalpha((int)*two)) || isalpha((int)*one) ) { ret = -1; } else { ret = 1; -- 1.7.6
On Sat, Aug 20, 2011 at 10:42:54PM -0400, Dave Reisner wrote:
vercmp exhibited some odd behavior here, declaring versions such as '23.3a' and '23.3.a' as equivalent. Add an extra check before we return equality that the versions have the same length.
Adds a pair of test cases to the vercmp test suite as well.
Reported-by: Eric Belanger <eric@archlinux.org> Signed-off-by: Dave Reisner <dreisner@archlinux.org> ---
And I've actually got this backwards... 23.3a should be older than 23.3.a. d
Not sure how or why some of this differed, but it is easy enough to set it back to how it was so it is easier to diff. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/version.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/libalpm/version.c b/lib/libalpm/version.c index 9f3a9b7..73d6a66 100644 --- a/lib/libalpm/version.c +++ b/lib/libalpm/version.c @@ -102,12 +102,12 @@ static int rpmvercmp(const char *a, const char *b) two = str2; /* loop through each version segment of str1 and str2 and compare them */ - while(*one && *two) { - while(*one && !isalnum((int)*one)) one++; - while(*two && !isalnum((int)*two)) two++; + while (*one && *two) { + while (*one && !isalnum((int)*one)) one++; + while (*two && !isalnum((int)*two)) two++; /* If we ran to the end of either, we are finished with the loop */ - if(!(*one && *two)) break; + if (!(*one && *two)) break; ptr1 = one; ptr2 = two; @@ -115,13 +115,13 @@ static int rpmvercmp(const char *a, const char *b) /* grab first completely alpha or completely numeric segment */ /* leave one and two pointing to the start of the alpha or numeric */ /* segment and walk ptr1 and ptr2 to end of segment */ - if(isdigit((int)*ptr1)) { - while(*ptr1 && isdigit((int)*ptr1)) ptr1++; - while(*ptr2 && isdigit((int)*ptr2)) ptr2++; + if (isdigit((int)*ptr1)) { + while (*ptr1 && isdigit((int)*ptr1)) ptr1++; + while (*ptr2 && isdigit((int)*ptr2)) ptr2++; isnum = 1; } else { - while(*ptr1 && isalpha((int)*ptr1)) ptr1++; - while(*ptr2 && isalpha((int)*ptr2)) ptr2++; + while (*ptr1 && isalpha((int)*ptr1)) ptr1++; + while (*ptr2 && isalpha((int)*ptr2)) ptr2++; isnum = 0; } -- 1.7.6
We had this interesting set of facts conundrum, according to vercmp return values: 2.0a < 2.0 2.0 < 2.0.a 2.0a == 2.0.a This introduces a code change that ensures '2.0a < 2.0.a' as would be expected by the first two comparisons. Unfortunately this stays us a bit further from upstream RPM code, but those are the breaks (in RPM, the versions involving 'a' do in fact compare the same, but they are both greater than the bare '2.0'). Signed-off-by: Dan McGee <dan@archlinux.org> --- doc/pacman.8.txt | 2 +- doc/vercmp.8.txt | 2 +- lib/libalpm/version.c | 9 +++++++-- test/util/vercmptest.sh | 6 ++++++ 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index 2a640f8..5985381 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -86,7 +86,7 @@ You can also use `pacman -Su` to upgrade all packages that are out of date. See to determine which packages need upgrading. This behavior operates as follows: Alphanumeric: - 1.0a < 1.0alpha < 1.0b < 1.0beta < 1.0p < 1.0pre < 1.0rc < 1.0 + 1.0a < 1.0b < 1.0beta < 1.0p < 1.0pre < 1.0rc < 1.0 < 1.0.a < 1.0.1 Numeric: 1 < 1.0 < 1.1 < 1.1.1 < 1.2 < 2.0 < 3.0.0 + diff --git a/doc/vercmp.8.txt b/doc/vercmp.8.txt index a3bc561..4b0490f 100644 --- a/doc/vercmp.8.txt +++ b/doc/vercmp.8.txt @@ -26,7 +26,7 @@ numbers. It outputs values as follows: Version comparsion operates as follows: Alphanumeric: - 1.0a < 1.0alpha < 1.0b < 1.0beta < 1.0p < 1.0pre < 1.0rc < 1.0 + 1.0a < 1.0b < 1.0beta < 1.0p < 1.0pre < 1.0rc < 1.0 < 1.0.a < 1.0.1 Numeric: 1 < 1.0 < 1.1 < 1.1.1 < 1.2 < 2.0 < 3.0.0 diff --git a/lib/libalpm/version.c b/lib/libalpm/version.c index 73d6a66..6b65a41 100644 --- a/lib/libalpm/version.c +++ b/lib/libalpm/version.c @@ -98,8 +98,8 @@ static int rpmvercmp(const char *a, const char *b) str1 = strdup(a); str2 = strdup(b); - one = str1; - two = str2; + one = ptr1 = str1; + two = ptr2 = str2; /* loop through each version segment of str1 and str2 and compare them */ while (*one && *two) { @@ -109,6 +109,11 @@ static int rpmvercmp(const char *a, const char *b) /* If we ran to the end of either, we are finished with the loop */ if (!(*one && *two)) break; + /* If the separator lengths were different, we are also finished */ + if ((one - ptr1) != (two - ptr2)) { + return (one - ptr1) < (two - ptr2) ? -1 : 1; + } + ptr1 = one; ptr2 = two; diff --git a/test/util/vercmptest.sh b/test/util/vercmptest.sh index 54ede04..6b4bcbc 100755 --- a/test/util/vercmptest.sh +++ b/test/util/vercmptest.sh @@ -118,6 +118,12 @@ runtest 1.5.1 1.5.b 1 runtest 1.5.b-1 1.5.b 0 runtest 1.5-1 1.5.b -1 +# same/similar content, differing separators +runtest 2.0 2_0 0 +runtest 2.0_a 2_0.a 0 +runtest 2.0a 2.0.a -1 +runtest 2___a 2_a 1 + # epoch included version comparisons runtest 0:1.0 0:1.0 0 runtest 0:1.0 0:1.1 -1 -- 1.7.6
participants (3)
-
Dan McGee
-
Dan McGee
-
Dave Reisner