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 <miod.vallat@powerdns.com>

diff --git a/pdns/dnsname.cc b/pdns/dnsname.cc
index f09f2b1e5f55546587601a8615472d7ce5661495..31767710a0b4d688410de18e74dce438f9381e71 100644 (file)
--- 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 934a757296a2147e8576326593f8f59ee7f5a7b5..68f36eff9bed5e32d3e8fc8583f24e69488025c8 100644 (file)
--- 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 a478e59bade78a03fad2cdaade6d17841a3771d1..25523822b1fb5270331338dd0de55b4720401534 100644 (file)
--- 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<b));
-  BOOST_CHECK(!a.canonCompare(b));
+  BOOST_CHECK(a.canonCompare_three_way(b) == 0);
 }
 
 BOOST_AUTO_TEST_CASE(test_casing) {
@@ -702,11 +702,11 @@ BOOST_AUTO_TEST_CASE(test_casing) {
 
 BOOST_AUTO_TEST_CASE(test_compare_canonical) {
   DNSName lower("bert.com."), higher("alpha.nl.");
-  BOOST_CHECK(lower.canonCompare(higher));
+  BOOST_CHECK(lower.canonCompare_three_way(higher) < 0);
 
-  BOOST_CHECK(DNSName("bert.com").canonCompare(DNSName("www.bert.com")));
-  BOOST_CHECK(DNSName("BeRt.com").canonCompare(DNSName("WWW.berT.com")));
-  BOOST_CHECK(!DNSName("www.BeRt.com").canonCompare(DNSName("WWW.berT.com")));
+  BOOST_CHECK(DNSName("bert.com").canonCompare_three_way(DNSName("www.bert.com")) < 0);
+  BOOST_CHECK(DNSName("BeRt.com").canonCompare_three_way(DNSName("WWW.berT.com")) < 0);
+  BOOST_CHECK(DNSName("www.BeRt.com").canonCompare_three_way(DNSName("WWW.berT.com")) == 0);
 
   CanonDNSNameCompare a;
   BOOST_CHECK(a(g_rootdnsname, DNSName("www.powerdns.com")));