Cleanups and commenting.

Change-Id: I491a68af8614198af450942cd378c9486f27d338

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<char*>(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<uint64_t*>(&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<std::size_t>(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<char*>(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);