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

[Oleksiy Vyalov]

Looks good - I ran your patch on Ubuntu, no regression.

Just minor things.


================
Comment at: include/lldb/Host/posix/PipePosix.h:34
@@ -49,10 +33,3 @@
 
-    int
-    GetReadFileDescriptor() const;
-    
-    int
-    GetWriteFileDescriptor() const;
-    
-    // Close both descriptors
-    void
-    Close();
+    virtual Error CreateNew(bool child_process_inherit) override;
+    virtual Error CreateNew(llvm::StringRef name, bool child_process_inherit) override;
----------------
You may follow the same style here as you did in PipeWindows - no need to have virtual prefix.

================
Comment at: include/lldb/Host/posix/PipePosix.h:55
@@ -77,1 +54,3 @@
 private:
+  void CloseReadFileDescriptor();
+  void CloseWriteFileDescriptor();
----------------
It seems an indentation issue - could you fix it?

================
Comment at: source/Host/posix/ConnectionFileDescriptorPosix.cpp:297
@@ -295,1 +296,3 @@
+    Error result = m_pipe.Write("i", 1, bytes_written);
+    return result.Success();
 }
----------------
return (result.Success() && bytes_written == 1) ?

================
Comment at: source/Host/posix/ConnectionFileDescriptorPosix.cpp:337
@@ -333,3 +336,3 @@
                 log->Printf("%p ConnectionFileDescriptor::Disconnect(): Couldn't get the lock, sent 'q' to %d, result = %d.",
-                            static_cast<void *>(this), m_pipe.GetWriteFileDescriptor(), result);
+                            static_cast<void *>(this), m_pipe.GetWriteFileDescriptor(), bytes_written);
         }
----------------
Do we need to log result variable instead of bytes_written?

http://reviews.llvm.org/D6686

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