Issue 1794 - add handling of std::vector<std::string> by Communication::{Input,Output}Buffer

src/shared/Communication.h

 #ifndef COMMUNICATION_H
 #define COMMUNICATION_H
 
 #include <memory>
 #include <sstream>
 #include <stdexcept>
 #include <stdint.h>
 #include <string>
 #include <vector>
 #include <Windows.h>
@@ skipping 20 matching lines @@
     PROC_SET_PREF,
     PROC_GET_PREF,
     PROC_IS_FIRST_RUN_ACTION_NEEDED,
     PROC_CHECK_FOR_UPDATES,
     PROC_GET_DOCUMENTATION_LINK,
     PROC_TOGGLE_PLUGIN_ENABLED,
     PROC_GET_HOST,
     PROC_COMPARE_VERSIONS
   };
   enum ValueType : uint32_t {
-    TYPE_PROC, TYPE_STRING, TYPE_WSTRING, TYPE_INT64, TYPE_INT32, TYPE_BOOL
+    TYPE_PROC, TYPE_STRING, TYPE_WSTRING, TYPE_INT64, TYPE_INT32, TYPE_BOOL, TYPE_STRINGS
   };
   typedef uint32_t SizeType;
 
   class InputBuffer
   {
   public:
     InputBuffer() : buffer(), hasType(false) {}
     InputBuffer(const std::string& data) : buffer(data), hasType(false) {}
     InputBuffer(const InputBuffer& copy) 
     { 
       hasType = copy.hasType; 
       buffer = std::istringstream(copy.buffer.str()); 
       currentType = copy.currentType; 
     }
     InputBuffer& operator>>(ProcType& value) { return Read(value, TYPE_PROC); }
     InputBuffer& operator>>(std::string& value) { return ReadString(value, TYPE_STRING); }
     InputBuffer& operator>>(std::wstring& value) { return ReadString(value, TYPE_WSTRING); }
     InputBuffer& operator>>(int64_t& value) { return Read(value, TYPE_INT64); }
     InputBuffer& operator>>(int32_t& value) { return Read(value, TYPE_INT32); }
     InputBuffer& operator>>(bool& value) { return Read(value, TYPE_BOOL); }
+    InputBuffer& operator>>(std::vector<std::string>& value) { return ReadStrings(value); }
     InputBuffer& operator=(const InputBuffer& copy) 
     { 
       hasType = copy.hasType; 
       buffer = std::istringstream(copy.buffer.str()); 
       currentType = copy.currentType; 
       return *this; 
     }
     ValueType GetType();
   private:
     std::istringstream buffer;
     ValueType currentType;
     bool hasType;
 
     void CheckType(ValueType expectedType);
 
     template<class T>
     InputBuffer& ReadString(T& value, ValueType expectedType)
     {
       CheckType(expectedType);
 
       SizeType length;
       ReadBinary(length);
 
       std::auto_ptr<T::value_type> data(new T::value_type[length]);
       buffer.read(reinterpret_cast<char*>(data.get()), sizeof(T::value_type) * length);

[Eric, 2015/02/09 16:18:57]

This should be 'const_cast'.

It should also be documented here that we're reading into the constant internal
buffer of a string. As we've discussed before, this happens to work but is not
guaranteed to work. A note should appear here acknowledging that.

[sergei, 2015/02/09 16:55:37]

Here we are not reading the data into the constant buffer of the string, though
instead of `std::auto_ptr<T::value_type>` here should be
`std::unique_ptr<T::value_type[]>`, so this particular line is correct.

[Eric, 2015/02/09 17:34:02]

Sorry. Too hasty about 'string'.

Nevertheless, using 'reinterpret_cast' would be totally unnecessary if we were
always transmitting UTF-8 across the pipe. We'd always get 'string' out of it,
and then we'd either assign it or convert it as appropriate.

       if (buffer.fail())
         throw new std::runtime_error("Unexpected end of input buffer");
 
       value.assign(data.get(), length);

[Eric, 2015/02/09 16:18:57]

This is the place to use either plain assignment or a UTF conversion function.

       return *this;
     }
 
+    InputBuffer& ReadStrings(std::vector<std::string>& value)
+    {
+      value.clear();
+      CheckType(ValueType::TYPE_STRINGS);
+
+      SizeType length;
+      ReadBinary(length);
+      value.resize(length);
+      for (SizeType i = 0; i < length; ++i)
+      {
+        operator>>(value[i]);
+      }
+      return *this;
+    }
+
     template<class T>
     InputBuffer& Read(T& value, ValueType expectedType)
     {
       CheckType(expectedType);
       ReadBinary(value);
       return *this;
     }
 
     template<class T>
     void ReadBinary(T& value)
@@ skipping 16 matching lines @@
     std::string Get()
     {
       return buffer.str();
     }
     OutputBuffer& operator<<(ProcType value) { return Write(value, TYPE_PROC); }
     OutputBuffer& operator<<(const std::string& value) { return WriteString(value, TYPE_STRING); }
     OutputBuffer& operator<<(const std::wstring& value) { return WriteString(value, TYPE_WSTRING); }
     OutputBuffer& operator<<(int64_t value) { return Write(value, TYPE_INT64); }
     OutputBuffer& operator<<(int32_t value) { return Write(value, TYPE_INT32); }
     OutputBuffer& operator<<(bool value) { return Write(value, TYPE_BOOL); }
+    OutputBuffer& operator<<(const std::vector<std::string>& value) { return WriteStrings(value); }
   private:
     std::ostringstream buffer;
 
     // Disallow copying
     const OutputBuffer& operator=(const OutputBuffer&);
 
     template<class T>
     OutputBuffer& WriteString(const T& value, ValueType type)
     {
       WriteBinary(type);
 
       SizeType length = static_cast<SizeType>(value.size());
       WriteBinary(length);
 
       buffer.write(reinterpret_cast<const char*>(value.c_str()), sizeof(T::value_type) * length);

[Eric, 2015/02/09 16:18:57]

Please take the opportunity to change this to 'const_cast'

[sergei, 2015/02/09 16:55:37]

We should not change this to `const_cast`, here, as well as above,
`reinterpret_cast` is used because we are reading and writing bytes.

       if (buffer.fail())
         throw new std::runtime_error("Unexpected error writing to output buffer");
 
       return *this;
     }
 
+    OutputBuffer& WriteStrings(const std::vector<std::string>& value)
+    {
+      WriteBinary(ValueType::TYPE_STRINGS);
+      WriteBinary(static_cast<SizeType>(value.size()));
+      for (const auto& str : value)
+      {
+        operator<<(str);
+      }
+      return *this;
+    }
+
     template<class T>
     OutputBuffer& Write(const T value, ValueType type)
     {
       WriteBinary(type);
       WriteBinary(value);
       return *this;
     }
 
     template<class T>
     void WriteBinary(const T& value)
@@ skipping 32 matching lines @@
 
     InputBuffer ReadMessage();
     void WriteMessage(OutputBuffer& message);
 
   protected:
     HANDLE pipe;
   };
 }
 
 #endif