From 7e730d8f7f7af927a52cd1b7668c64b4c82daa4a Mon Sep 17 00:00:00 2001 From: David Gobbi Date: Mon, 20 Feb 2023 10:24:45 -0700 Subject: Tests: Add cases for cmSystemTools::VersionCompare --- Tests/CMakeLib/testSystemTools.cxx | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/Tests/CMakeLib/testSystemTools.cxx b/Tests/CMakeLib/testSystemTools.cxx index 754205e..1d5db1c 100644 --- a/Tests/CMakeLib/testSystemTools.cxx +++ b/Tests/CMakeLib/testSystemTools.cxx @@ -36,6 +36,35 @@ int testSystemTools(int /*unused*/, char* /*unused*/[]) "cmSystemTools::UpperCase"); // ---------------------------------------------------------------------- + // Test cmSystemTools::VersionCompare + cmAssert(cmSystemTools::VersionCompareEqual("", ""), + "VersionCompareEqual empty string"); + cmAssert(!cmSystemTools::VersionCompareGreater("", ""), + "VersionCompareGreater empty string"); + cmAssert(cmSystemTools::VersionCompareEqual("1", "1a"), + "VersionCompareEqual letters"); + cmAssert(!cmSystemTools::VersionCompareGreater("1", "1a"), + "VersionCompareGreater letters"); + cmAssert(cmSystemTools::VersionCompareEqual("001", "1"), + "VersionCompareEqual leading zeros equal"); + cmAssert(!cmSystemTools::VersionCompareGreater("001", "1"), + "VersionCompareGreater leading zeros equal"); + cmAssert(!cmSystemTools::VersionCompareEqual("002", "1"), + "VersionCompareEqual leading zeros greater"); + cmAssert(cmSystemTools::VersionCompareGreater("002", "1"), + "VersionCompareGreater leading zeros greater"); + cmAssert(!cmSystemTools::VersionCompareEqual("6.2.1", "6.3.1"), + "VersionCompareEqual components less"); + cmAssert(!cmSystemTools::VersionCompareGreater("6.2.1", "6.3.1"), + "VersionCompareGreater components less"); + cmAssert(!cmSystemTools::VersionCompareEqual("6.2.1", "6.2"), + "VersionCompareEqual different length"); + cmAssert(cmSystemTools::VersionCompareGreater("6.2.1", "6.2"), + "VersionCompareGreater different length"); + cmAssert(cmSystemTools::VersionCompareGreater("3.141592653", "3.14159265"), + "VersionCompareGreater more digits"); + + // ---------------------------------------------------------------------- // Test cmSystemTools::strverscmp cmAssert(cmSystemTools::strverscmp("", "") == 0, "strverscmp empty string"); cmAssert(cmSystemTools::strverscmp("abc", "") > 0, -- cgit v0.12 From aa86e8ddfd6b9dc12e54aeb7e5b5c1503c272c42 Mon Sep 17 00:00:00 2001 From: David Gobbi Date: Mon, 20 Feb 2023 08:59:11 -0700 Subject: Remove component size limit for version comparisons The VersionCompare() function converted version components to 'unsigned long' prior to comparing them. Any version components too large for 'unsigned long' were treated as equal to ULONG_MAX. This impacted operators like VERSION_GREATER, VERSION_LESS, and VERSION_EQUAL. The new code does not limit the length of the version components for valid comparisons. --- Source/cmSystemTools.cxx | 35 ++++++++++++++++++++++++++++------- Tests/CMakeLib/testSystemTools.cxx | 22 ++++++++++++++++++++-- 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index f34e35f..3d61270 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -3197,20 +3197,41 @@ bool VersionCompare(cmSystemTools::CompareOp op, const char* lhss, { const char* endl = lhss; const char* endr = rhss; - unsigned long lhs; - unsigned long rhs; while (((*endl >= '0') && (*endl <= '9')) || ((*endr >= '0') && (*endr <= '9'))) { - // Do component-wise comparison. - lhs = strtoul(endl, const_cast(&endl), 10); - rhs = strtoul(endr, const_cast(&endr), 10); + // Do component-wise comparison, ignoring leading zeros + // (components are treated as integers, not as mantissas) + while (*endl == '0') { + endl++; + } + while (*endr == '0') { + endr++; + } + + const char* beginl = endl; + const char* beginr = endr; + + // count significant digits + while ((*endl >= '0') && (*endl <= '9')) { + endl++; + } + while ((*endr >= '0') && (*endr <= '9')) { + endr++; + } + + // compare number of digits first + ptrdiff_t r = ((endl - beginl) - (endr - beginr)); + if (r == 0) { + // compare the digits if number of digits is equal + r = strncmp(beginl, beginr, endl - beginl); + } - if (lhs < rhs) { + if (r < 0) { // lhs < rhs, so true if operation is LESS return (op & cmSystemTools::OP_LESS) != 0; } - if (lhs > rhs) { + if (r > 0) { // lhs > rhs, so true if operation is GREATER return (op & cmSystemTools::OP_GREATER) != 0; } diff --git a/Tests/CMakeLib/testSystemTools.cxx b/Tests/CMakeLib/testSystemTools.cxx index 1d5db1c..0f0c025 100644 --- a/Tests/CMakeLib/testSystemTools.cxx +++ b/Tests/CMakeLib/testSystemTools.cxx @@ -61,8 +61,26 @@ int testSystemTools(int /*unused*/, char* /*unused*/[]) "VersionCompareEqual different length"); cmAssert(cmSystemTools::VersionCompareGreater("6.2.1", "6.2"), "VersionCompareGreater different length"); - cmAssert(cmSystemTools::VersionCompareGreater("3.141592653", "3.14159265"), - "VersionCompareGreater more digits"); + cmAssert( + !cmSystemTools::VersionCompareEqual( + "3.14159265358979323846264338327950288419716939937510582097494459230", + "3.14159265358979323846264338327950288419716939937510582097494459231"), + "VersionCompareEqual long number"); + cmAssert( + !cmSystemTools::VersionCompareGreater( + "3.14159265358979323846264338327950288419716939937510582097494459230", + "3.14159265358979323846264338327950288419716939937510582097494459231"), + "VersionCompareGreater long number"); + cmAssert( + !cmSystemTools::VersionCompareEqual( + "3.141592653589793238462643383279502884197169399375105820974944592307", + "3.14159265358979323846264338327950288419716939937510582097494459231"), + "VersionCompareEqual more digits"); + cmAssert( + cmSystemTools::VersionCompareGreater( + "3.141592653589793238462643383279502884197169399375105820974944592307", + "3.14159265358979323846264338327950288419716939937510582097494459231"), + "VersionCompareGreater more digits"); // ---------------------------------------------------------------------- // Test cmSystemTools::strverscmp -- cgit v0.12