pdns_ilexicographical_compare() and dns_iequals() optimization

diff --git a/pdns/misc.hh b/pdns/misc.hh
index 4da76e684df62fe3a854752e8a11e98460b6ba34..3a665fc11340113a0e92ea18c738e8fb1abdbea1 100644 (file)
--- a/pdns/misc.hh
+++ b/pdns/misc.hh
@@ -311,36 +311,35 @@ inline bool operator<(const struct timeval& lhs, const struct timeval& rhs)
 }
 
 inline bool pdns_ilexicographical_compare(const std::string& a, const std::string& b)  __attribute__((pure));
-inline bool pdns_ilexicographical_compare(const std::string& a, const std::string& b) 
+inline bool pdns_ilexicographical_compare(const std::string& a, const std::string& b)
 {
-  string::size_type aLen = a.length(), bLen = b.length(), n;
   const unsigned char *aPtr = (const unsigned char*)a.c_str(), *bPtr = (const unsigned char*)b.c_str();
-  int result;
-  
-  for(n = 0 ; n < aLen && n < bLen ; ++n) {
-      if((result = dns_tolower(*aPtr++) - dns_tolower(*bPtr++))) {
-        return result < 0;
-      }
+
+  while(*aPtr && *bPtr) {
+    if ((*aPtr != *bPtr) && (dns_tolower(*aPtr) - dns_tolower(*bPtr)))
+      return (dns_tolower(*aPtr) - dns_tolower(*bPtr)) < 0;
+    aPtr++;
+    bPtr++;
   }
-  if(n == aLen && n == bLen) // strings are equal (in length)
-    return 0; 
-  if(n == aLen) // first string was shorter
-    return true; 
-  return false;
+  if(!*aPtr && !*bPtr) // strings are equal (in length)
+    return false;
+  return !*aPtr; // true if first string was shorter
 }
 
 inline bool pdns_iequals(const std::string& a, const std::string& b) __attribute__((pure));
-
-inline bool pdns_iequals(const std::string& a, const std::string& b) 
+inline bool pdns_iequals(const std::string& a, const std::string& b)
 {
-  string::size_type aLen = a.length(), bLen = b.length(), n;
+  if (a.length() != b.length())
+    return false;
+
   const char *aPtr = a.c_str(), *bPtr = b.c_str();
-  
-  for(n = 0 ; n < aLen && n < bLen ; ++n) {
-      if(dns_tolower(*aPtr++) != dns_tolower(*bPtr++))
-        return false;
+  while(*aPtr) {
+    if((*aPtr != *bPtr) && (dns_tolower(*aPtr) != dns_tolower(*bPtr)))
+      return false;
+    aPtr++;
+    bPtr++;
   }
-  return aLen == bLen; // strings are equal (in length)
+  return true;
 }
 
 // lifted from boost, with thanks

diff --git a/pdns/test-misc_hh.cc b/pdns/test-misc_hh.cc
index d50201e6515e325070c454c7f6a1520c204dec16..0e7f37b70fc6828585c3a547dea2186ffb725276 100644 (file)
--- a/pdns/test-misc_hh.cc
+++ b/pdns/test-misc_hh.cc
@@ -2,6 +2,9 @@
 #define BOOST_TEST_NO_MAIN
 
 #include <boost/test/unit_test.hpp>
+#include <boost/assign/list_of.hpp>
+#include <boost/foreach.hpp>
+#include <boost/tuple/tuple.hpp>
 #include "misc.hh"
 #include "dns.hh"
 #include <utility>
@@ -12,7 +15,7 @@ BOOST_AUTO_TEST_SUITE(misc_hh)
 typedef pair<std::string, uint16_t> typedns_t;
 
 BOOST_AUTO_TEST_CASE(test_CIStringCompare) {
-       set<std::string, CIStringCompare> nsset;  
+       set<std::string, CIStringCompare> nsset;
        nsset.insert("abc");
        nsset.insert("ns.example.com");
        nsset.insert("");
@@ -29,7 +32,7 @@ BOOST_AUTO_TEST_CASE(test_CIStringCompare) {
 }
 
 BOOST_AUTO_TEST_CASE(test_CIStringPairCompare) {
-       set<typedns_t, CIStringPairCompare> nsset2;  
+       set<typedns_t, CIStringPairCompare> nsset2;
        nsset2.insert(make_pair("ns.example.com", 1));
        nsset2.insert(make_pair("abc", 1));
        nsset2.insert(make_pair("", 1));
@@ -37,9 +40,8 @@ BOOST_AUTO_TEST_CASE(test_CIStringPairCompare) {
        nsset2.insert(make_pair("abc", 2));
        nsset2.insert(make_pair("abc", 1));
        nsset2.insert(make_pair("ns.example.com", 0));
-    nsset2.insert(make_pair("abc", 2));
+       nsset2.insert(make_pair("abc", 2));
        nsset2.insert(make_pair("ABC", 2));
-
        BOOST_CHECK_EQUAL(nsset2.size(), 6);
 
        ostringstream s;
@@ -49,8 +51,57 @@ BOOST_AUTO_TEST_CASE(test_CIStringPairCompare) {
        BOOST_CHECK_EQUAL(s.str(), "(|1)(abc|1)(abc|2)(def|1)(ns.example.com|0)(ns.example.com|1)");
 }
 
+BOOST_AUTO_TEST_CASE(test_pdns_ilexicographical_compare) {
+  typedef boost::tuple<const std::string, const std::string, bool> case_t;
+  typedef std::list<case_t> cases_t;
+
+  cases_t cases = boost::assign::list_of
+    (case_t(std::string(""), std::string(""), false))
+    (case_t(std::string(""), std::string("abc"), true))
+    (case_t(std::string("abc"), std::string(""), false))
+    (case_t(std::string("abc"), std::string("abcd"), true))
+    (case_t(std::string("abcd"), std::string("abc"), false))
+    (case_t(std::string("abd"), std::string("abc"), false))
+    (case_t(std::string("abc"), std::string("abd"), true))
+    (case_t(std::string("abc"), std::string("Abc"), false))
+    (case_t(std::string("Abc"), std::string("abc"), false))
+  ;
+
+  BOOST_FOREACH(const case_t& val, cases) {
+    bool res;
+    res = pdns_ilexicographical_compare(val.get<0>(), val.get<1>());
+    BOOST_CHECK_EQUAL(res, val.get<2>());
+  }
+}
+
+BOOST_AUTO_TEST_CASE(test_pdns_iequals) {
+  typedef boost::tuple<const std::string, const std::string, bool> case_t;
+  typedef std::list<case_t> cases_t;
+
+  cases_t cases = boost::assign::list_of
+    (case_t(std::string(""), std::string(""), true))
+    (case_t(std::string(""), std::string("abc"), false))
+    (case_t(std::string("abc"), std::string(""), false))
+    (case_t(std::string("abc"), std::string("abcd"), false))
+    (case_t(std::string("abcd"), std::string("abc"), false))
+    (case_t(std::string("abd"), std::string("abc"), false))
+    (case_t(std::string("abc"), std::string("abd"), false))
+    (case_t(std::string("abc"), std::string("Abc"), true))
+    (case_t(std::string("Abc"), std::string("abc"), true))
+  ;
+
+  BOOST_FOREACH(const case_t& val, cases) {
+    bool res;
+    res = pdns_iequals(val.get<0>(), val.get<1>());
+    BOOST_CHECK_EQUAL(res, val.get<2>());
+  }
+}
+
 BOOST_AUTO_TEST_CASE(test_stripDot) {
-    BOOST_CHECK_EQUAL(stripDot("www.powerdns.com."), "www.powerdns.com");
+  BOOST_CHECK_EQUAL(stripDot("."), "");
+  BOOST_CHECK_EQUAL(stripDot(""), "");
+  BOOST_CHECK_EQUAL(stripDot("www.powerdns.com."), "www.powerdns.com");
+  BOOST_CHECK_EQUAL(stripDot("www.powerdns.com"), "www.powerdns.com");
 }
 
 BOOST_AUTO_TEST_CASE(test_labelReverse) {