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); } Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56234/new/ https://reviews.llvm.org/D56234 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits