From 22c72955f5b7d5fdfa00c98dd01db6dcd7ba1381 Mon Sep 17 00:00:00 2001 From: Matias Fontanini Date: Sun, 30 Apr 2017 09:25:57 -0700 Subject: [PATCH] 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 | 143 ++++++++++++-------------------------- src/CMakeLists.txt | 1 + src/hw_address.cpp | 88 +++++++++++++++++++++++ 3 files changed, 134 insertions(+), 98 deletions(-) create mode 100644 src/hw_address.cpp diff --git a/include/tins/hw_address.h b/include/tins/hw_address.h index fde80d4..853822a 100644 --- a/include/tins/hw_address.h +++ b/include/tins/hw_address.h @@ -31,15 +31,32 @@ #define TINS_HWADDRESS_H #include -#include -#include -#include -#include -#include -#include +#include +#include +#include #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 +template 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 broadcast; + static const HWAddress 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 HWAddress(const char (&address)[i]) { - convert(address, buffer_); + Internals::string_to_hw_address(address, buffer_, n); } /** @@ -163,18 +178,15 @@ public: */ template HWAddress(const HWAddress& 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(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 - static void convert(const std::string& hw_addr, OutputIterator output); - static HWAddress make_broadcast_address() { // Build a buffer made of n 0xff bytes uint8_t buffer[n]; @@ -375,65 +376,11 @@ private: return HWAddress(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 -template -void HWAddress::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 -const HWAddress HWAddress::broadcast = make_broadcast_address(); +template +const HWAddress HWAddress::broadcast = make_broadcast_address(); } // namespace Tins diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 6f1dd91..04d069b 100644 --- a/src/CMakeLists.txt +++ b/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 diff --git a/src/hw_address.cpp b/src/hw_address.cpp new file mode 100644 index 0000000..93780c8 --- /dev/null +++ b/src/hw_address.cpp @@ -0,0 +1,88 @@ +#include +#include +#include +#include +#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