From: Miod Vallat Date: Thu, 26 Jun 2025 07:33:22 +0000 (+0200) Subject: Introduce canonCompare_three_way for {DNS,Zone}Name. X-Git-Tag: rec-5.3.0-alpha2~40^2~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=04ca63229445fb8a41c9c563e8cc4ad6e6b054cf;p=thirdparty%2Fpdns.git Introduce canonCompare_three_way for {DNS,Zone}Name. This allows ZoneName::canonCompare* to only invoke DNSName::canonCompare_three_way() once instead of DNSName::canonCompare() twice. Signed-off-by: Miod Vallat --- diff --git a/pdns/dnsname.cc b/pdns/dnsname.cc index f09f2b1e5f..31767710a0 100644 --- a/pdns/dnsname.cc +++ b/pdns/dnsname.cc @@ -462,13 +462,35 @@ void DNSName::prependRawLabel(const std::string& label) d_storage = prep+d_storage; } -bool DNSName::slowCanonCompare(const DNSName& rhs) const +int DNSName::slowCanonCompare_three_way(const DNSName& rhs) const { - auto ours=getRawLabels(), rhsLabels = rhs.getRawLabels(); - return std::lexicographical_compare(ours.rbegin(), ours.rend(), rhsLabels.rbegin(), rhsLabels.rend(), CIStringCompare()); + // Unfortunately we can't use std::lexicographical_compare_three_way() yet + // as this would require C++20. + const auto ours = getRawLabels(); + const auto rhsLabels = rhs.getRawLabels(); + auto iter1 = ours.rbegin(); + const auto& last1 = ours.rend(); + auto iter2 = rhsLabels.rbegin(); + const auto& last2 = rhsLabels.rend(); + while (iter1 != last1 && iter2 != last2) { + if (int rc = pdns_ilexicographical_compare_three_way(*iter1, *iter2); rc != 0) { + return rc; + } + ++iter1; // NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic) + ++iter2; // NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic) + } + if (iter1 == last1) { + if (iter2 != last2) { + return -1; // lt + } + } + else { + return 1; // gt + } + return 0; // eq } -bool DNSName::canonCompare(const DNSName& rhs) const +int DNSName::canonCompare_three_way(const DNSName& rhs) const { // 01234567890abcd // us: 1a3www4ds9a2nl @@ -487,14 +509,16 @@ bool DNSName::canonCompare(const DNSName& rhs) const rhspos[rhscount++]=(p-(const unsigned char*)rhs.d_storage.c_str()); if(ourcount == sizeof(ourpos) || rhscount==sizeof(rhspos)) { - return slowCanonCompare(rhs); + return slowCanonCompare_three_way(rhs); } for(;;) { - if(ourcount == 0 && rhscount != 0) - return true; - if(rhscount == 0) - return false; + if(ourcount == 0 && rhscount != 0) { + return -1; // lt + } + if(rhscount == 0) { + return ourcount == 0 ? 0 /* eq */ : 1 /* gt */; + } ourcount--; rhscount--; @@ -506,10 +530,9 @@ bool DNSName::canonCompare(const DNSName& rhs) const rhs.d_storage.c_str() + rhspos[rhscount] + 1, *(rhs.d_storage.c_str() + rhspos[rhscount]))); if (res != 0) { - return res < 0; + return res; } } - return false; } @@ -932,20 +955,14 @@ bool ZoneName::operator<(const ZoneName& rhs) const return d_variant < rhs.d_variant; } -bool ZoneName::canonCompare(const ZoneName& rhs) const +int ZoneName::canonCompare_three_way(const ZoneName& rhs) const { // Similarly to operator< above, this compares DNSName first, variant - // second. Unfortunately because DNSName::canonCompare() is complicated, - // it can't pragmatically be duplicated here, hence the two calls. - // TODO: change DNSName::canonCompare() to return a three-state value - // (lt, eq, ge) in order to be able to call it only once. - if (!d_name.canonCompare(rhs.d_name)) { - return false; - } - if (!rhs.d_name.canonCompare(d_name)) { - return true; + // second. + if (int rc = d_name.canonCompare_three_way(rhs.d_name); rc != 0) { + return rc; } // Both DNSName compare equal. - return d_variant < rhs.d_variant; + return d_variant.compare(rhs.d_variant); } #endif // ] diff --git a/pdns/dnsname.hh b/pdns/dnsname.hh index 934a757296..68f36eff9b 100644 --- a/pdns/dnsname.hh +++ b/pdns/dnsname.hh @@ -188,8 +188,9 @@ public: rhs.d_storage.rbegin(), rhs.d_storage.rend(), DNSNameCompare()); } - bool canonCompare(const DNSName& rhs) const; - bool slowCanonCompare(const DNSName& rhs) const; + int slowCanonCompare_three_way(const DNSName& rhs) const; + int canonCompare_three_way(const DNSName& rhs) const; + inline bool canonCompare(const DNSName& rhs) const { return canonCompare_three_way(rhs) < 0; } typedef boost::container::string string_t; @@ -334,7 +335,8 @@ public: bool operator<(const ZoneName& rhs) const; - bool canonCompare(const ZoneName& rhs) const; + int canonCompare_three_way(const ZoneName& rhs) const; + inline bool canonCompare(const ZoneName& rhs) const { return canonCompare_three_way(rhs) < 0; } // Conversion from ZoneName to DNSName explicit operator const DNSName&() const { return d_name; } diff --git a/pdns/test-dnsname_cc.cc b/pdns/test-dnsname_cc.cc index a478e59bad..25523822b1 100644 --- a/pdns/test-dnsname_cc.cc +++ b/pdns/test-dnsname_cc.cc @@ -24,8 +24,8 @@ BOOST_AUTO_TEST_CASE(test_basic) { DNSName aroot("a.root-servers.net"), broot("b.root-servers.net"); BOOST_CHECK(aroot < broot); BOOST_CHECK(!(broot < aroot)); - BOOST_CHECK(aroot.canonCompare(broot)); - BOOST_CHECK(!broot.canonCompare(aroot)); + BOOST_CHECK(aroot.canonCompare_three_way(broot) < 0); + BOOST_CHECK(broot.canonCompare_three_way(aroot) > 0); string before("www.ds9a.nl."); @@ -684,7 +684,7 @@ BOOST_AUTO_TEST_CASE(test_compare_naive) { BOOST_AUTO_TEST_CASE(test_compare_empty) { DNSName a, b; BOOST_CHECK(!(a