[Lldb-commits] [PATCH] Enhance the Pipe interface for better portability
Oleksiy Vyalov
ovyalov at google.com
Tue Dec 16 14:10:22 PST 2014
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/
More information about the lldb-commits
mailing list