[Lldb-commits] [PATCH] Enhance the Pipe interface for better portability

[Oleksiy Vyalov]

Please see my comments.


================
Comment at: include/lldb/Host/PipeBase.h:27
@@ +26,3 @@
+    virtual Error CreateNew(bool child_process_inherit) = 0;
+    virtual Error CreateNew(bool child_process_inherit, llvm::StringRef name) = 0;
+    virtual Error OpenAsReader(bool child_process_inherit, llvm::StringRef name) = 0;
----------------
Minor thing - you may have name passed as a first parameter so a signature will be in accord with open sys call syntax - open(name, flags) 

================
Comment at: source/Host/posix/PipePosix.cpp:66
@@ -58,3 +65,3 @@
     if (::pipe2(m_fds, (child_processes_inherit) ? 0 : O_CLOEXEC) == 0)
-        return true;
+        return error;
 #else
----------------
Please add error.SetErrorToErrno();

================
Comment at: source/Host/posix/PipePosix.cpp:76
@@ -68,2 +75,3 @@
                 Close();
-                return false;
+                error.SetErrorToErrno();
+                return error;
----------------
Could you call error.SetErrorToErrno(); before Close() to make sure that Close doesn't overwrite errno?

================
Comment at: source/Host/windows/PipeWindows.cpp:116
@@ -78,3 +115,3 @@
     {
-        m_read_overlapped = new OVERLAPPED;
-        ZeroMemory(m_read_overlapped, sizeof(OVERLAPPED));
+        m_read = ::CreateFile(name.data(), GENERIC_READ, 0, &attributes, OPEN_EXISTING, FILE_FLAG_OVERLAPPED, NULL);
+        if (INVALID_HANDLE_VALUE == m_read)
----------------
Just out of curiosity - why are you using std::string::data here, not c_str?

http://reviews.llvm.org/D6686

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/