Moved all pipe functionality into a self-containing Communication::Pipe class

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>
 
 namespace Communication
 {
   extern const std::wstring pipeName;

[Felix Dahlke, 2013/06/03 12:18:47]

Since we're really always using the same pipeName, I guess it makes sense to
have it as a private constant and not passing a name in. I don't see a need for
having more than one pipe.

[Wladimir Palant, 2013/06/03 14:00:26]

The tests use a different pipe name.

-  const int bufferSize = 1024;

[Felix Dahlke, 2013/06/03 12:18:47]

This is only being used in Communication.cpp, I'd rather have it as an
implementation-only variable.

[Wladimir Palant, 2013/06/03 14:00:26]

True.

 
-  enum ValueType {TYPE_STRING, TYPE_WSTRING, TYPE_INT64, TYPE_INT32, TYPE_BOOL};
+  enum ProcType : uint32_t {
+    PROC_MATCHES,
+    PROC_GET_ELEMHIDE_SELECTORS,
+    PROC_AVAILABLE_SUBSCRIPTIONS,
+    PROC_LISTED_SUBSCRIPTIONS,
+    PROC_SET_SUBSCRIPTION,
+    PROC_UPDATE_ALL_SUBSCRIPTIONS,
+    PROC_GET_EXCEPTION_DOMAINS,
+    PROC_ADD_FILTER,
+  };
+  enum ValueType : uint32_t {
+    TYPE_PROC, TYPE_STRING, TYPE_WSTRING, TYPE_INT64, TYPE_INT32, TYPE_BOOL
+  };
+  typedef uint32_t SizeType;
 
   class InputBuffer
   {
   public:
-    InputBuffer(const std::string& data) : buffer(data) {}
+    InputBuffer(const std::string& data) : buffer(data), hasType(false) {}
+    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); }
   private:
     std::istringstream buffer;
-    int32_t currentType;

[Felix Dahlke, 2013/06/03 12:18:47]

I'd like to have the typedef here I asked for in the other review.

[Wladimir Palant, 2013/06/03 14:00:26]

You are getting it in the other review :)

+    ValueType currentType;
     bool hasType;
 
-    void CheckType(ValueType expectedType)

[Felix Dahlke, 2013/06/03 12:18:47]

I'd prefer to see this function in the cpp, it's a bit big for an inline
function IMO.

-    {
-      if (!hasType)
-        ReadBinary(currentType);
-
-      if (currentType != expectedType)
-      {
-        // Make sure we don't attempt to read the type again
-        hasType = true;
-        throw new std::runtime_error("Unexpected type found in input buffer");
-      }
-      else
-        hasType = false;
-    }
+    void CheckType(ValueType expectedType);
 
     template<class T>
     InputBuffer& ReadString(T& value, ValueType expectedType)
     {
       CheckType(expectedType);
 
-      uint32_t length;
+      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);
       if (buffer.fail())
         throw new std::runtime_error("Unexpected end of input buffer");
 
       value.assign(data.get(), length);
       return *this;
     }
@@ skipping 20 matching lines @@
   public:
     OutputBuffer() {}
 
     // Explicit copy constructor to allow returning OutputBuffer by value
     OutputBuffer(const OutputBuffer& copy) : buffer(copy.buffer.str()) {}
 
     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); }
   private:
     std::ostringstream buffer;
 
+    // Disallow copying
+    const OutputBuffer& operator=(const OutputBuffer&);
+
     template<class T>
-    OutputBuffer& WriteString(const T& value, int32_t type)
+    OutputBuffer& WriteString(const T& value, ValueType type)
     {
       WriteBinary(type);
 
-      uint32_t length = value.size();
+      SizeType length = value.size();
       WriteBinary(length);
 
       buffer.write(reinterpret_cast<const char*>(value.c_str()), sizeof(T::value_type) * length);
       if (buffer.fail())
         throw new std::runtime_error("Unexpected error writing to output buffer");
 
       return *this;
     }
 
     template<class T>
-    OutputBuffer& Write(const T value, int32_t type)
+    OutputBuffer& Write(const T value, ValueType type)
     {
       WriteBinary(type);
       WriteBinary(value);
       return *this;
     }
 
     template<class T>
     void WriteBinary(const T& value)
     {
       buffer.write(reinterpret_cast<const char*>(&value), sizeof(T));
@@ skipping 18 matching lines @@
 
     InputBuffer ReadMessage();
     void WriteMessage(OutputBuffer& message);
 
   protected:
     HANDLE pipe;
   };
 }
 
 #endif