diff --git a/src/SequenceNumber.cpp b/src/SequenceNumber.cpp index 91a9bda128..33c497d012 100644 --- a/src/SequenceNumber.cpp +++ b/src/SequenceNumber.cpp @@ -10,19 +10,19 @@ namespace ntimpl { bool operator<(const SequenceNumber& lhs, const SequenceNumber& rhs) { - if (lhs.m_val < rhs.m_val) - return (rhs.m_val - lhs.m_val) < (1u << 15); - else if (lhs.m_val > rhs.m_val) - return (lhs.m_val - rhs.m_val) > (1u << 15); + if (lhs.m_value < rhs.m_value) + return (rhs.m_value - lhs.m_value) < (1u << 15); + else if (lhs.m_value > rhs.m_value) + return (lhs.m_value - rhs.m_value) > (1u << 15); else return false; } bool operator>(const SequenceNumber& lhs, const SequenceNumber& rhs) { - if (lhs.m_val < rhs.m_val) - return (rhs.m_val - lhs.m_val) > (1u << 15); - else if (lhs.m_val > rhs.m_val) - return (lhs.m_val - rhs.m_val) < (1u << 15); + if (lhs.m_value < rhs.m_value) + return (rhs.m_value - lhs.m_value) > (1u << 15); + else if (lhs.m_value > rhs.m_value) + return (lhs.m_value - rhs.m_value) < (1u << 15); else return false; } diff --git a/src/SequenceNumber.h b/src/SequenceNumber.h index 9f9f5d50a5..9469752393 100644 --- a/src/SequenceNumber.h +++ b/src/SequenceNumber.h @@ -10,14 +10,15 @@ namespace ntimpl { +/* A sequence number per RFC 1982 */ class SequenceNumber { public: - explicit SequenceNumber(unsigned int val) : m_val(val) {} - unsigned int GetVal() const { return m_val; } + explicit SequenceNumber(unsigned int value) : m_value(value) {} + unsigned int value() const { return m_value; } SequenceNumber& operator++() { - ++m_val; - if (m_val > 0xffff) m_val = 0; + ++m_value; + if (m_value > 0xffff) m_value = 0; return *this; } SequenceNumber operator++(int) { @@ -34,7 +35,7 @@ class SequenceNumber { friend bool operator!=(const SequenceNumber& lhs, const SequenceNumber& rhs); private: - unsigned int m_val; + unsigned int m_value; }; bool operator<(const SequenceNumber& lhs, const SequenceNumber& rhs); @@ -49,11 +50,11 @@ inline bool operator>=(const SequenceNumber& lhs, const SequenceNumber& rhs) { } inline bool operator==(const SequenceNumber& lhs, const SequenceNumber& rhs) { - return lhs.m_val == rhs.m_val; + return lhs.m_value == rhs.m_value; } inline bool operator!=(const SequenceNumber& lhs, const SequenceNumber& rhs) { - return lhs.m_val != rhs.m_val; + return lhs.m_value != rhs.m_value; } } // namespace ntimpl diff --git a/src/WireDecoder.cpp b/src/WireDecoder.cpp index 99bd453401..9e0e5c2ce3 100644 --- a/src/WireDecoder.cpp +++ b/src/WireDecoder.cpp @@ -17,7 +17,8 @@ using namespace ntimpl; -static double read_double(char*& buf) { +static double ReadDouble(char*& buf) { + // Fast but non-portable! std::uint64_t val = (*((unsigned char*)buf)) & 0xff; ++buf; val <<= 8; @@ -45,8 +46,10 @@ static double read_double(char*& buf) { } WireDecoder::WireDecoder(raw_istream& is, unsigned int proto_rev) : m_is(is) { + // Start with a 1K temporary buffer. Use malloc instead of new so we can + // realloc. m_allocated = 1024; - m_buf = (char*)std::malloc(m_allocated); + m_buf = static_cast(std::malloc(m_allocated)); m_proto_rev = proto_rev; m_error = nullptr; } @@ -56,21 +59,23 @@ WireDecoder::~WireDecoder() { std::free(m_buf); } bool WireDecoder::ReadDouble(double* val) { char* buf; if (!Read(&buf, 8)) return false; - *val = read_double(buf); + *val = ::ReadDouble(buf); return true; } void WireDecoder::Realloc(std::size_t len) { + // Double current buffer size until we have enough space. if (m_allocated >= len) return; std::size_t newlen = m_allocated * 2; while (newlen < len) newlen *= 2; - m_buf = (char*)std::realloc(m_buf, newlen); + m_buf = static_cast(std::realloc(m_buf, newlen)); m_allocated = newlen; } bool WireDecoder::ReadType(NT_Type* type) { unsigned int itype; if (!Read8(&itype)) return false; + // Convert from byte value to enum switch (itype) { case 0x00: *type = NT_BOOLEAN; @@ -137,7 +142,8 @@ bool WireDecoder::ReadValue(NT_Type type, NT_Value* value) { // array values char* buf; if (!Read(&buf, size)) return false; - value->data.arr_boolean.arr = (int*)std::malloc(size * sizeof(int)); + value->data.arr_boolean.arr = + static_cast(std::malloc(size * sizeof(int))); for (unsigned int i = 0; i < size; ++i) value->data.arr_boolean.arr[i] = buf[i] ? 1 : 0; break; @@ -151,9 +157,10 @@ bool WireDecoder::ReadValue(NT_Type type, NT_Value* value) { // array values char* buf; if (!Read(&buf, size * 8)) return false; - value->data.arr_double.arr = (double*)std::malloc(size * sizeof(double)); + value->data.arr_double.arr = + static_cast(std::malloc(size * sizeof(double))); for (unsigned int i = 0; i < size; ++i) - value->data.arr_double.arr[i] = read_double(buf); + value->data.arr_double.arr[i] = ::ReadDouble(buf); break; } case NT_STRING_ARRAY: { @@ -164,7 +171,7 @@ bool WireDecoder::ReadValue(NT_Type type, NT_Value* value) { // array values value->data.arr_string.arr = - (NT_String*)std::malloc(size * sizeof(NT_String)); + static_cast(std::malloc(size * sizeof(NT_String))); for (unsigned int i = 0; i < size; ++i) { if (!ReadString(&value->data.arr_string.arr[i])) { // cleanup to avoid memory leaks @@ -194,12 +201,12 @@ bool WireDecoder::ReadString(NT_String* str) { if (!ReadUleb128(&v)) return false; str->len = v; } - str->str = (char*)std::malloc(str->len + 1); + str->str = static_cast(std::malloc(str->len + 1)); // +1 for nul if (!m_is.read(str->str, str->len)) { std::free(str->str); str->str = 0; return false; } - str->str[str->len] = '\0'; + str->str[str->len] = '\0'; // be nice and nul-terminate it return true; } diff --git a/src/WireDecoder.h b/src/WireDecoder.h index 9e48760602..619a634e5b 100644 --- a/src/WireDecoder.h +++ b/src/WireDecoder.h @@ -16,6 +16,13 @@ namespace ntimpl { +/* Decodes network data into native representation. + * This class is designed to read from a raw_istream, which provides a blocking + * read interface. There are no provisions in this class for resuming a read + * that was interrupted partway. Read functions return false if + * raw_istream.read() returned false (indicating the end of the input data + * stream). + */ class WireDecoder { public: explicit WireDecoder(raw_istream& is, unsigned int proto_rev); @@ -23,16 +30,25 @@ class WireDecoder { void set_proto_rev(unsigned int proto_rev) { m_proto_rev = proto_rev; } + /* Clears error indicator. */ void Reset() { m_error = nullptr; } + /* Returns error indicator (a string describing the error). Returns nullptr + * if no error has occurred. + */ const char* error() const { return m_error; } + /* Reads the specified number of bytes. + * @param buf pointer to read data (output parameter) + * @param len number of bytes to read + */ bool Read(char** buf, std::size_t len) { if (len > m_allocated) Realloc(len); *buf = m_buf; return m_is.read(m_buf, len); } + /* Reads a single byte. */ bool Read8(unsigned int* val) { char* buf; if (!Read(&buf, 1)) return false; @@ -40,6 +56,7 @@ class WireDecoder { return true; } + /* Reads a 16-bit word. */ bool Read16(unsigned int* val) { char* buf; if (!Read(&buf, 2)) return false; @@ -51,6 +68,7 @@ class WireDecoder { return true; } + /* Reads a 32-bit word. */ bool Read32(unsigned long* val) { char* buf; if (!Read(&buf, 4)) return false; @@ -68,8 +86,10 @@ class WireDecoder { return true; } + /* Reads a double. */ bool ReadDouble(double* val); + /* Reads an ULEB128-encoded unsigned integer. */ bool ReadUleb128(unsigned long* val) { return ntimpl::ReadUleb128(m_is, val); } @@ -82,15 +102,23 @@ class WireDecoder { WireDecoder& operator=(const WireDecoder&) = delete; protected: + /* The protocol revision. E.g. 0x0200 for version 2.0. */ unsigned int m_proto_rev; + + /* Error indicator. */ const char* m_error; private: + /* Reallocate temporary buffer to specified length. */ void Realloc(std::size_t len); + /* input stream */ raw_istream& m_is; + /* temporary buffer */ char* m_buf; + + /* allocated size of temporary buffer */ std::size_t m_allocated; }; diff --git a/src/WireEncoder.cpp b/src/WireEncoder.cpp index 8a71c19441..7866821144 100644 --- a/src/WireEncoder.cpp +++ b/src/WireEncoder.cpp @@ -18,7 +18,9 @@ using namespace ntimpl; WireEncoder::WireEncoder(unsigned int proto_rev) { - m_start = m_cur = (char*)std::malloc(1024); + // Start with a 1024-byte buffer. Use malloc instead of new so we can + // realloc. + m_start = m_cur = static_cast(std::malloc(1024)); m_end = m_start + 1024; m_proto_rev = proto_rev; m_error = nullptr; @@ -28,6 +30,7 @@ WireEncoder::~WireEncoder() { std::free(m_start); } void WireEncoder::WriteDouble(double val) { Reserve(8); + // The highest performance way to do this, albeit non-portable. std::uint64_t v = *reinterpret_cast(&val); *m_cur++ = (char)((v >> 56) & 0xff); *m_cur++ = (char)((v >> 48) & 0xff); @@ -40,12 +43,16 @@ void WireEncoder::WriteDouble(double val) { } void WireEncoder::ReserveSlow(std::size_t len) { - assert(m_end > m_cur); + assert(m_end >= m_cur); + // should never happen due to checks in Reserve(), but check anyway if (static_cast(m_end - m_cur) >= len) return; + + // Double current buffer size until we have enough space. Since we're + // reserving space, it's likely more will be required soon in any case. std::size_t pos = m_cur - m_start; std::size_t newlen = (m_end - m_start) * 2; while (newlen < (pos + len)) newlen *= 2; - m_start = (char*)std::realloc(m_start, newlen); + m_start = static_cast(std::realloc(m_start, newlen)); m_cur = m_start + pos; m_end = m_start + newlen; } @@ -57,6 +64,7 @@ void WireEncoder::WriteUleb128(unsigned long val) { void WireEncoder::WriteType(NT_Type type) { Reserve(1); + // Convert from enum to actual byte value. switch (type) { case NT_BOOLEAN: *m_cur = 0x00; @@ -97,7 +105,7 @@ void WireEncoder::WriteType(NT_Type type) { ++m_cur; } -std::size_t WireEncoder::GetValueSize(const NT_Value& value) { +std::size_t WireEncoder::GetValueSize(const NT_Value& value) const { switch (value.type) { case NT_BOOLEAN: return 1; @@ -109,13 +117,23 @@ std::size_t WireEncoder::GetValueSize(const NT_Value& value) { case NT_RPC: if (m_proto_rev < 0x0300u) return 0; return GetStringSize(value.data.v_raw); - case NT_BOOLEAN_ARRAY: - return 1 + value.data.arr_boolean.size; - case NT_DOUBLE_ARRAY: - return 1 + value.data.arr_double.size * 8; + case NT_BOOLEAN_ARRAY: { + // 1-byte size, 1 byte per element + std::size_t size = value.data.arr_boolean.size; + if (size > 0xff) size = 0xff; // size is only 1 byte, truncate + return 1 + size; + } + case NT_DOUBLE_ARRAY: { + // 1-byte size, 8 bytes per element + std::size_t size = value.data.arr_double.size; + if (size > 0xff) size = 0xff; // size is only 1 byte, truncate + return 1 + size * 8; + } case NT_STRING_ARRAY: { - size_t len = 1; - for (size_t i = 0; i < value.data.arr_string.size; ++i) + std::size_t size = value.data.arr_string.size; + if (size > 0xff) size = 0xff; // size is only 1 byte, truncate + std::size_t len = 1; // 1-byte size + for (std::size_t i = 0; i < size; ++i) len += GetStringSize(value.data.arr_string.arr[i]); return len; } @@ -145,7 +163,7 @@ void WireEncoder::WriteValue(const NT_Value& value) { break; case NT_BOOLEAN_ARRAY: { std::size_t size = value.data.arr_boolean.size; - if (size > 0xff) size = 0xff; + if (size > 0xff) size = 0xff; // size is only 1 byte, truncate Reserve(1 + size); Write8(size); @@ -155,7 +173,7 @@ void WireEncoder::WriteValue(const NT_Value& value) { } case NT_DOUBLE_ARRAY: { std::size_t size = value.data.arr_double.size; - if (size > 0xff) size = 0xff; + if (size > 0xff) size = 0xff; // size is only 1 byte, truncate Reserve(1 + size * 8); Write8(size); @@ -165,7 +183,7 @@ void WireEncoder::WriteValue(const NT_Value& value) { } case NT_STRING_ARRAY: { std::size_t size = value.data.arr_string.size; - if (size > 0xff) size = 0xff; + if (size > 0xff) size = 0xff; // size is only 1 byte, truncate Write8(size); for (std::size_t i = 0; i < size; ++i) @@ -178,8 +196,12 @@ void WireEncoder::WriteValue(const NT_Value& value) { } } -std::size_t WireEncoder::GetStringSize(const NT_String& str) { - if (m_proto_rev < 0x0300u) return 2 + str.len; +std::size_t WireEncoder::GetStringSize(const NT_String& str) const { + if (m_proto_rev < 0x0300u) { + std::size_t len = str.len; + if (len > 0xffff) len = 0xffff; // Limited to 64K length; truncate + return 2 + len; + } return SizeUleb128(str.len) + str.len; } @@ -187,8 +209,7 @@ void WireEncoder::WriteString(const NT_String& str) { // length std::size_t len = str.len; if (m_proto_rev < 0x0300u) { - // Limited to 64K length; truncate if necessary - if (len > 0xffff) len = 0xffff; + if (len > 0xffff) len = 0xffff; // Limited to 64K length; truncate Write16(len); } else WriteUleb128(len); diff --git a/src/WireEncoder.h b/src/WireEncoder.h index c462883e06..7d564fc64a 100644 --- a/src/WireEncoder.h +++ b/src/WireEncoder.h @@ -15,40 +15,59 @@ namespace ntimpl { +/* Encodes native data for network transmission. + * This class maintains an internal memory buffer for written data so that + * it can be efficiently bursted to the network after a number of writes + * have been performed. For this reason, all operations are non-blocking. + */ class WireEncoder { public: explicit WireEncoder(unsigned int proto_rev); ~WireEncoder(); + /* Change the protocol revision (mostly affects value encoding). */ void set_proto_rev(unsigned int proto_rev) { m_proto_rev = proto_rev; } + /* Clears buffer and error indicator. */ void Reset() { m_cur = m_start; m_error = nullptr; } + /* Returns error indicator (a string describing the error). Returns nullptr + * if no error has occurred. + */ const char* error() const { return m_error; } + /* Returns pointer to start of memory buffer with written data. */ const char* data() const { return m_start; } + /* Returns number of bytes written to memory buffer. */ std::size_t size() const { return m_cur - m_start; } + /* Ensures the buffer has sufficient space to write len bytes. Reallocates + * the buffer if necessary. + */ void Reserve(std::size_t len) { // assert(m_end > m_cur); - if (static_cast(m_end - m_cur) < len) ReserveSlow(len); + if (static_cast(m_end - m_cur) < len) + ReserveSlow(len); // Need to reallocate memory } + /* Writes a single byte. */ void Write8(unsigned int val) { Reserve(1); *m_cur++ = (char)(val & 0xff); } + /* Writes a 16-bit word. */ void Write16(unsigned int val) { Reserve(2); *m_cur++ = (char)((val >> 8) & 0xff); *m_cur++ = (char)(val & 0xff); } + /* Writes a 32-bit word. */ void Write32(unsigned long val) { Reserve(4); *m_cur++ = (char)((val >> 24) & 0xff); @@ -57,28 +76,47 @@ class WireEncoder { *m_cur++ = (char)(val & 0xff); } + /* Writes a double. */ void WriteDouble(double val); + /* Writes an ULEB128-encoded unsigned integer. */ void WriteUleb128(unsigned long val); + void WriteType(NT_Type type); void WriteValue(const NT_Value& value); void WriteString(const NT_String& str); - std::size_t GetValueSize(const NT_Value& value); - std::size_t GetStringSize(const NT_String& str); + /* Utility function to get the written size of a value (without actually + * writing it). + */ + std::size_t GetValueSize(const NT_Value& value) const; + + /* Utility function to get the written size of a string (without actually + * writing it). + */ + std::size_t GetStringSize(const NT_String& str) const; WireEncoder(const WireEncoder&) = delete; WireEncoder& operator=(const WireEncoder&) = delete; protected: + /* The protocol revision. E.g. 0x0200 for version 2.0. */ unsigned int m_proto_rev; + + /* Error indicator. */ const char* m_error; private: + /* The slow path for Reserve() when we need to reallocate memory. */ void ReserveSlow(std::size_t len); + /* The start of the memory buffer. */ char* m_start; + + /* Where to write the next byte. */ char* m_cur; + + /* The end of the allocated buffer. */ char* m_end; }; diff --git a/src/base64.cpp b/src/base64.cpp index 3d7bf4997d..479b11a881 100644 --- a/src/base64.cpp +++ b/src/base64.cpp @@ -118,7 +118,7 @@ std::size_t Base64Decode(char *bufplain, const char *bufcoded) { nprbytes -= 4; } - // Note: (nprbytes == 1) would be an error, so just ingore that case + // Note: (nprbytes == 1) would be an error, so just ignore that case if (nprbytes > 1) { *(bufout++) = (unsigned char)(pr2six[*bufin] << 2 | pr2six[bufin[1]] >> 4); }