Remove Storage template parameter from HWAddress, move impl to cpp

This is a breaking ABI change. This might break some forward
declarations and hopefully no one was actually using the
Storage type for anything.

include/tins/hw_address.h

@@ -31,15 +31,32 @@
 #define TINS_HWADDRESS_H
 
 #include <stdint.h>
-#include <stdexcept>
-#include <iterator>
-#include <algorithm>
-#include <iomanip>
-#include <iostream>
-#include <sstream>
+#include <iosfwd>
+#include <string>
+#include <cstring>
 #include "cxxstd.h"
 
 namespace Tins {
+namespace Internals {
+
+// Defined in hw_address.cpp
+/**
+ * \cond
+ */
+std::string hw_address_to_string(const uint8_t* ptr, size_t count);
+
+void string_to_hw_address(const std::string& hw_addr, uint8_t* output, size_t output_size);
+
+bool hw_address_equal_compare(const uint8_t* start1, const uint8_t* end1,
+                              const uint8_t* start2);
+
+bool hw_address_lt_compare(const uint8_t* start1, const uint8_t* end1,
+                           const uint8_t* start2, const uint8_t* end2);
+
+/**
+ * \endcond
+ */
+} // Internals
 
 /**
  * \class HWAddress
@@ -61,15 +78,13 @@ namespace Tins {
  * }
  * \endcode
  */
-template<size_t n, typename Storage = uint8_t>
+template<size_t n>
 class HWAddress {
 public:
     /**
      * \brief The type of the elements stored in the hardware address.
-     * 
-     * This is the same as the template parameter Storage.
      */
-    typedef Storage storage_type;
+    typedef uint8_t storage_type;
     
     /**
      * \brief The random access iterator type.
@@ -90,7 +105,7 @@ public:
     /**
      * \brief The broadcast address.
      */
-    static const HWAddress<n, Storage> broadcast;
+    static const HWAddress<n> broadcast;
     
     /**
      * \brief Constructor from a const storage_type*.
@@ -109,10 +124,10 @@ public:
      */
     HWAddress(const storage_type* ptr = 0) {
         if (ptr) {
-            std::copy(ptr, ptr + address_size, buffer_);
+            std::memcpy(buffer_, ptr, address_size);
         }
         else {
-            std::fill(begin(), end(), storage_type());
+            std::memset(buffer_, 0, address_size);
         }
     }
     
@@ -128,7 +143,7 @@ public:
      * \param address The hex-notation address to be parsed.
      */
     HWAddress(const std::string& address) {
-        convert(address, buffer_);
+        Internals::string_to_hw_address(address, buffer_, n);
     }
     
     /**
@@ -146,7 +161,7 @@ public:
      */
     template<size_t i>
     HWAddress(const char (&address)[i]) {
-        convert(address, buffer_);
+        Internals::string_to_hw_address(address, buffer_, n);
     }
     
     /**
@@ -163,18 +178,15 @@ public:
      */
     template<size_t i>
     HWAddress(const HWAddress<i>& rhs) {
-        // Fill extra bytes
-        std::fill(
-            // Copy as most as we can
-            std::copy(
-                rhs.begin(),
-                rhs.begin() + std::min(i, n),
-                begin()
-            ),
-            end(),
-            0
-        );
-        
+        size_t copy_threshold = i < n ? i : n;
+        for (size_t index = 0; index < n; ++index) {
+            if (index < copy_threshold) {
+                buffer_[index] = rhs[index];
+            }
+            else {
+                buffer_[index] = storage_type();
+            }
+        }
     }
     
     /**
@@ -225,7 +237,7 @@ public:
      * \return bool indicating whether addresses are equal.
      */
     bool operator==(const HWAddress& rhs) const {
-        return std::equal(begin(), end(), rhs.begin());
+        return Internals::hw_address_equal_compare(begin(), end(), rhs.begin());
     }
     
     /**
@@ -247,7 +259,7 @@ public:
      * \return bool indicating whether this address is less-than rhs.
      */
     bool operator<(const HWAddress& rhs) const {
-        return std::lexicographical_compare(begin(), end(), rhs.begin(), rhs.end());
+        return Internals::hw_address_lt_compare(begin(), end(), rhs.begin(), rhs.end());
     }
 
     /**
@@ -300,9 +312,7 @@ public:
      * \return std::string containing the hex-notation address.
      */
     std::string to_string() const {
-        std::ostringstream oss;
-        oss <<* this;
-        return oss.str();
+        return Internals::hw_address_to_string(buffer_, size());
     }
 
     /**
@@ -331,13 +341,7 @@ public:
      * \return std::ostream& pointing to the os parameter.
      */
     friend std::ostream& operator<<(std::ostream& os, const HWAddress& addr) {
-        std::transform(
-            addr.begin(), 
-            addr.end() - 1,
-            std::ostream_iterator<std::string>(os, ":"),
-            &HWAddress::storage_to_string
-        );
-        return os << storage_to_string(addr.begin()[HWAddress::address_size - 1]);
+        return os << addr.to_string();
     }
     
     /**
@@ -363,9 +367,6 @@ public:
         return output;
     }
 private:
-    template<typename OutputIterator>
-    static void convert(const std::string& hw_addr, OutputIterator output);
-    
     static HWAddress<n> make_broadcast_address() {
         // Build a buffer made of n 0xff bytes
         uint8_t buffer[n];
@@ -375,65 +376,11 @@ private:
         return HWAddress<n>(buffer);
     }
 
-    static std::string storage_to_string(storage_type element) {
-        std::ostringstream oss;
-        oss << std::hex;
-        if (element < 0x10) {
-            oss << '0';
-        }
-        oss << (unsigned)element;
-        return oss.str();
-    }
-
     storage_type buffer_[n];
 };
 
-template<size_t n, typename Storage>
-template<typename OutputIterator>
-void HWAddress<n, Storage>::convert(const std::string& hw_addr, 
-                                    OutputIterator output)  {
-    unsigned i(0);
-    size_t count(0);
-    storage_type tmp;
-    while (i < hw_addr.size() && count < n) {
-        const unsigned end = i+2;
-        tmp = storage_type();
-        while (i < end) {
-            if (hw_addr[i] >= 'a' && hw_addr[i] <= 'f') {
-                tmp = (tmp << 4) | (hw_addr[i] - 'a' + 10);
-            }
-            else if (hw_addr[i] >= 'A' && hw_addr[i] <= 'F') {
-                tmp = (tmp << 4) | (hw_addr[i] - 'A' + 10);
-            }
-            else if (hw_addr[i] >= '0' && hw_addr[i] <= '9') {
-                tmp = (tmp << 4) | (hw_addr[i] - '0');
-            }
-            else if (hw_addr[i] == ':') {
-                break;
-            }
-            else {
-                throw std::runtime_error("Invalid byte found");
-            }
-            i++;
-        }
-        *(output++) = tmp;
-        count++;
-        if (i < hw_addr.size()) {
-            if (hw_addr[i] == ':') {
-                i++;
-            }
-            else {
-                throw std::runtime_error("Invalid separator");
-            }
-        }
-    }
-    while (count++ < n) {
-        *(output++) = storage_type();
-    }
-}
-
-template<size_t n, typename Storage>
-const HWAddress<n, Storage> HWAddress<n, Storage>::broadcast = make_broadcast_address();
+template<size_t n>
+const HWAddress<n> HWAddress<n>::broadcast = make_broadcast_address();
 
 } // namespace Tins
 

src/CMakeLists.txt

@@ -28,6 +28,7 @@ set(SOURCES
     dot1q.cpp
     eapol.cpp
     ethernetII.cpp
+    hw_address.cpp
     icmp_extension.cpp
     icmp.cpp
     icmpv6.cpp

src/hw_address.cpp

@@ -0,0 +1,88 @@
+#include <sstream>
+#include <iomanip>
+#include <stdexcept>
+#include <algorithm>
+#include "hw_address.h"
+
+using std::string;
+using std::ostream;
+using std::hex;
+using std::ostringstream;
+using std::lexicographical_compare;
+using std::equal;
+
+namespace Tins {
+namespace Internals {
+
+void storage_to_string(ostream& output, uint8_t value) {
+    output << hex;
+    if (value < 0x10) {
+        output << '0';
+    }
+    output << (unsigned)value;
+}
+
+string hw_address_to_string(const uint8_t* ptr, size_t count) {
+    ostringstream output;
+    for (size_t i = 0; i < count; ++i) {
+        if (i != 0) {
+            output << ":";
+        }
+        storage_to_string(output, ptr[i]);
+    }
+    return output.str();
+}
+
+void string_to_hw_address(const string& hw_addr, uint8_t* output, size_t output_size)  {
+    unsigned i = 0;
+    size_t count = 0;
+    uint8_t tmp;
+    while (i < hw_addr.size() && count < output_size) {
+        const unsigned end = i+2;
+        tmp = 0;
+        while (i < end) {
+            if (hw_addr[i] >= 'a' && hw_addr[i] <= 'f') {
+                tmp = (tmp << 4) | (hw_addr[i] - 'a' + 10);
+            }
+            else if (hw_addr[i] >= 'A' && hw_addr[i] <= 'F') {
+                tmp = (tmp << 4) | (hw_addr[i] - 'A' + 10);
+            }
+            else if (hw_addr[i] >= '0' && hw_addr[i] <= '9') {
+                tmp = (tmp << 4) | (hw_addr[i] - '0');
+            }
+            else if (hw_addr[i] == ':') {
+                break;
+            }
+            else {
+                throw std::runtime_error("Invalid byte found");
+            }
+            i++;
+        }
+        *(output++) = tmp;
+        count++;
+        if (i < hw_addr.size()) {
+            if (hw_addr[i] == ':') {
+                i++;
+            }
+            else {
+                throw std::runtime_error("Invalid separator");
+            }
+        }
+    }
+    while (count++ < output_size) {
+        *(output++) = 0;
+    }
+}
+
+bool hw_address_equal_compare(const uint8_t* start1, const uint8_t* end1,
+                              const uint8_t* start2) {
+    return equal(start1, end1, start2);
+}
+
+bool hw_address_lt_compare(const uint8_t* start1, const uint8_t* end1,
+                           const uint8_t* start2, const uint8_t* end2) {
+    return lexicographical_compare(start1, end1, start2, end2);
+}
+
+} // Internals
+} // Tins