[Lldb-commits] [PATCH] D56234: [lldb-server] Add unnamed pipe support to PipeWindows

Hui Huang via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Jan 9 14:43:08 PST 2019

Hui added inline comments.

Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:1051
 // and fill the data into "command_output_ptr"
 #if defined(__APPLE__)
         // Binding to port zero, we need to figure out what port it ends up
My first attempt was to enable windows specific along with apple's since named pipe is already supported. Seemed there was some problem for client pipe to open a server pipe on the same machine. This might not be the scope of this discussion. 

Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:1074
+        }
+        auto write_handle = socket_pipe.GetWriteNativeHandle();
+        debugserver_args.AppendArgument(llvm::StringRef("--pipe"));
zturner wrote:
> labath wrote:
> > What do you think about adding `GetWriteNativeHandle` to the generic Pipe interface? In posix the file descriptor _is_ the native handle, so PipePosix could just return the fd, but it would allow you to write this bit of code generically without ifdefs (well, maybe except the `AppendCloseFileAction` below). Similarly, we could have a generic Pipe factory function(*) which takes the handles as input, which would allow you to implement the lldb-server side of things without ifdefs.
> > 
> > (*) I think a factory function (`Pipe::FromNativeHandles`?) would be better than a constructor to avoid ambiguities between descriptors and handles.
> That's one possibility, but at the very least we should at least move the `CreateNew` and logging out of the ifdef.
Based on the lldb::pipe_t, using one of the introduced overloads could just put the windows specific all the way into the existing non-apple implementation.   AppendCloseFileAction seemed yet impact anything.

lldb::pipe_t GetReadPipe() const { return lldb::pipe_t(m_read); }
lldb::pipe_t GetWritePipe() const { return lldb::pipe_t(m_write); }




More information about the lldb-commits mailing list