extract filepipes to files

fix the file handle errors

FilePipe.cpp

@@ -0,0 +1,28 @@
+#include "stdafx.h"
+#include "FilePipe.h"
+
+#include "SystemError.h"
+
+Linter::FilePipe::Pipe Linter::FilePipe::create()
+{
+    SECURITY_ATTRIBUTES security;
+
+    security.nLength = sizeof(SECURITY_ATTRIBUTES);
+    security.bInheritHandle = TRUE;
+    security.lpSecurityDescriptor = nullptr;
+
+    HANDLE parent;
+    HANDLE child;
+    if (!CreatePipe(&parent, &child, &security, 0))
+    {
+        throw SystemError();
+    }
+
+    //Stop my handle being inherited by the child
+    if (!SetHandleInformation(parent, HANDLE_FLAG_INHERIT, 0))
+    {
+        throw SystemError();
+    }
+
+    return {HandleWrapper(parent), HandleWrapper(child)};
+}

FilePipe.h

@@ -0,0 +1,18 @@
+#pragma once
+
+#include "HandleWrapper.h"
+
+namespace Linter
+{
+    class FilePipe
+    {
+      public:
+        struct Pipe
+        {
+            HandleWrapper m_reader;
+            HandleWrapper m_writer;
+        };
+
+        static Pipe create();
+    };
+}    // namespace Linter

HandleWrapper.cpp

@@ -3,116 +3,91 @@
 
 #include "SystemError.h"
 
-#include <limits>
-#include <utility>
+using namespace Linter;
 
-namespace Linter
+HandleWrapper::HandleWrapper(HANDLE h) : m_handle(h)
 {
-    HandleWrapper::HandleWrapper(HANDLE h) : m_handle(h)
+    if (h == INVALID_HANDLE_VALUE)
     {
-        if (h == INVALID_HANDLE_VALUE)
-        {
-            throw SystemError();
-        }
+        throw SystemError();
     }
+}
 
-    HandleWrapper::HandleWrapper(HandleWrapper &&other) noexcept
-        : m_handle(std::exchange(other.m_handle, INVALID_HANDLE_VALUE))
-    {
-    }
+HandleWrapper::HandleWrapper(HandleWrapper &&other) noexcept : m_handle(std::exchange(other.m_handle, INVALID_HANDLE_VALUE))
+{
+}
 
-    void HandleWrapper::close()
+void HandleWrapper::close() const
+{
+    if (m_handle != INVALID_HANDLE_VALUE)
     {
-        if (m_handle != INVALID_HANDLE_VALUE)
+        HANDLE h{std::exchange(m_handle, INVALID_HANDLE_VALUE)};
+        if (!CloseHandle(h))
         {
-            HANDLE h{std::exchange(m_handle, INVALID_HANDLE_VALUE)};
-            if (!CloseHandle(h))
+            if (std::uncaught_exceptions() == 0)
             {
-                if (std::uncaught_exceptions() == 0)
-                {
-                    throw SystemError();
-                }
+                throw SystemError();
             }
         }
     }
+}
 
-    HandleWrapper::~HandleWrapper()
-    {
-        close();
-    }
+HandleWrapper::operator HANDLE() const noexcept
+{
+    return m_handle;
+}
 
-    void HandleWrapper::write_string(std::string const &str)
+HandleWrapper::~HandleWrapper()
+{
+    close();
+}
+
+void HandleWrapper::writeFile(std::string const &str) const
+{
+    char const *buff = str.c_str();
+    char const *end = buff + str.size();
+    while (buff != end)
     {
-        char const *buff = str.c_str();
-        char const *end = buff + str.size();
-        while (buff != end)
+        DWORD to_write = static_cast<DWORD>(std::min(static_cast<std::size_t>(std::numeric_limits<DWORD>::max()), str.size()));
+        DWORD written;
+        if (!WriteFile(m_handle, buff, to_write, &written, nullptr))
         {
-            DWORD to_write = static_cast<DWORD>(std::min(static_cast<std::size_t>(std::numeric_limits<DWORD>::max()), str.size()));
-            DWORD written;
-            if (!WriteFile(m_handle, buff, to_write, &written, nullptr))
-            {
-                //Bad things happened
-                throw SystemError();
-            }
-            buff += written;
+            //Bad things happened
+            throw SystemError();
         }
+        buff += written;
     }
+}
 
-    std::string HandleWrapper::read_file()
-    {
-        std::string result;
+std::string HandleWrapper::readFile() const
+{
+    std::string result;
 
-        static const DWORD buffer_size = 16384;
-        std::string buffer;
-        buffer.resize(buffer_size);
+    static const DWORD buffer_size = 16384;
+    std::string buffer;
+    buffer.resize(buffer_size);
 
-        for (;;)
+    for (;;)
+    {
+        DWORD readBytes;
+        //The API suggests when the other end closes the pipe, you should get 0. What appears to happen
+        //is that you get broken pipe.
+        if (!ReadFile(m_handle, &buffer[0], buffer_size, &readBytes, nullptr))
         {
-            DWORD readBytes;
-            //The API suggests when the other end closes the pipe, you should get 0. What appears to happen
-            //is that you get broken pipe.
-            if (!ReadFile(m_handle, &buffer[0], buffer_size, &readBytes, nullptr))
+            DWORD err = GetLastError();
+            if (err != ERROR_BROKEN_PIPE)
             {
-                DWORD err = GetLastError();
-                if (err != ERROR_BROKEN_PIPE)
-                {
-                    throw SystemError(err);
-                }
+                throw SystemError(err);
             }
-
-            if (readBytes == 0)
-            {
-                break;
-            }
-
-            result += std::string(&buffer[0], readBytes);
         }
 
-        return result;
-    }
-
-    std::pair<HandleWrapper, HandleWrapper> HandleWrapper::create_pipe()
-    {
-        SECURITY_ATTRIBUTES security;
-
-        security.nLength = sizeof(SECURITY_ATTRIBUTES);
-        security.bInheritHandle = TRUE;
-        security.lpSecurityDescriptor = nullptr;
-
-        HANDLE parent_h;
-        HANDLE child_h;
-        if (!CreatePipe(&parent_h, &child_h, &security, 0))
-        {
-            throw SystemError();
-        }
-
-        //Stop my handle being inherited by the child
-        if (!SetHandleInformation(parent_h, HANDLE_FLAG_INHERIT, 0))
+        if (readBytes == 0)
         {
-            throw SystemError();
+            break;
         }
 
-        return std::make_pair(HandleWrapper(parent_h), HandleWrapper(child_h));
+        result += std::string(&buffer[0], readBytes);
     }
 
-}    // namespace Linter
+    return result;
+}

HandleWrapper.h

@@ -1,47 +1,33 @@
 #pragma once
 
 #include <string>
-#include <utility>
-
-#include <wtypes.h>
 
 namespace Linter
 {
     class HandleWrapper
     {
       public:
         explicit HandleWrapper(HANDLE h);
-
         HandleWrapper(HandleWrapper const &) = delete;
-
         HandleWrapper(HandleWrapper &&other) noexcept;
-
         HandleWrapper &operator=(HandleWrapper const &) = delete;
-
         HandleWrapper &operator=(HandleWrapper &&other) = delete;
-
-        void close();
-
         ~HandleWrapper();
 
-        operator HANDLE() const noexcept
-        {
-            return m_handle;
-        }
+        void close() const;
+
+        operator HANDLE() const noexcept;
 
         /** Write a string to the handle
-         *
-         * @param str - string to write
-         */
-        void write_string(std::string const &str);
+        *
+        * @param str - string to write
+        */
+        void writeFile(std::string const &str) const;
 
         /** Read the entire file */
-        std::string read_file();
-
-        static std::pair<HandleWrapper, HandleWrapper> create_pipe();
+        std::string readFile() const;
 
       private:
-        HANDLE m_handle;
+        mutable HANDLE m_handle;
     };
-
 }    // namespace Linter

file.cpp

@@ -1,13 +1,12 @@
 #include "StdAfx.h"
 #include "file.h"
 
-#include "HandleWrapper.h"
+#include "FilePipe.h"
 #include "SystemError.h"
 
 #include <codecvt>
 
-using ::Linter::HandleWrapper;
-using ::Linter::SystemError;
+using namespace Linter;
 
 std::string File::exec(std::wstring commandLine, const nonstd::optional<std::string> &str)
 {
@@ -19,23 +18,15 @@ std::string File::exec(std::wstring commandLine, const nonstd::optional<std::str
         commandLine += '"';
     }
 
-    auto out_handles = HandleWrapper::create_pipe();
-    HandleWrapper &OUT_Rd{out_handles.first};
-    HandleWrapper &OUT_Wr{out_handles.second};
-
-    auto err_handles = HandleWrapper::create_pipe();
-    //HandleWrapper &ERR_Rd{err_handles.first}; //We don't use this?
-    HandleWrapper &ERR_Wr{err_handles.second};
-
-    auto in_handles = HandleWrapper::create_pipe();
-    HandleWrapper &IN_Wr{err_handles.first};
-    HandleWrapper &IN_Rd{err_handles.second};
+    const auto stdoutpipe = FilePipe::create();
+    const auto stderrpipe = FilePipe::create();
+    const auto stdinpipe = FilePipe::create();
 
     STARTUPINFO startInfo = {0};
     startInfo.cb = sizeof(STARTUPINFO);
-    startInfo.hStdError = ERR_Wr;
-    startInfo.hStdOutput = OUT_Wr;
-    startInfo.hStdInput = IN_Rd;
+    startInfo.hStdError = stderrpipe.m_writer;
+    startInfo.hStdOutput = stdoutpipe.m_writer;
+    startInfo.hStdInput = stdinpipe.m_reader;
     startInfo.dwFlags |= STARTF_USESTDHANDLES;
 
     PROCESS_INFORMATION procInfo = {0};
@@ -60,18 +51,18 @@ std::string File::exec(std::wstring commandLine, const nonstd::optional<std::str
 
     if (str.has_value())
     {
-        IN_Wr.write_string(str.value());
+        stdinpipe.m_writer.writeFile(str.value());
     }
 
     //We need to close all the handles for this end otherwise strange things happen.
     CloseHandle(procInfo.hProcess);
     CloseHandle(procInfo.hThread);
 
-    OUT_Wr.close();
-    ERR_Wr.close();
-    IN_Wr.close();
+    stdoutpipe.m_writer.close();
+    stderrpipe.m_writer.close();
+    stdinpipe.m_writer.close();
 
-    return OUT_Rd.read_file();
+    return stdoutpipe.m_reader.readFile();
 }
 
 File::File(const std::wstring &fileName, const std::wstring &directory) : m_fileName(fileName), m_directory(directory)
@@ -98,7 +89,7 @@ void File::write(const std::string &data)
     HandleWrapper fileHandle{
         CreateFile(tempFileName.c_str(), GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_TEMPORARY, NULL)};
 
-    fileHandle.write_string(data);
+    fileHandle.writeFile(data);
 
     m_file = tempFileName;
 }

linter.vcxproj

@@ -171,6 +171,7 @@
   <ItemGroup>
     <ClCompile Include="encoding.cpp" />
     <ClCompile Include="file.cpp" />
+    <ClCompile Include="FilePipe.cpp" />
     <ClCompile Include="HandleWrapper.cpp" />
     <ClCompile Include="linter.cpp" />
     <ClCompile Include="plugin.cpp" />
@@ -186,6 +187,7 @@
   <ItemGroup>
     <ClInclude Include="encoding.h" />
     <ClInclude Include="file.h" />
+    <ClInclude Include="FilePipe.h" />
     <ClInclude Include="HandleWrapper.h" />
     <ClInclude Include="linter.h" />
     <ClInclude Include="notepad\menuCmdID.h" />