compnerd added inline comments.
================ Comment at: include/lldb/Host/windows/PosixApi.h:108 + // To be implemented. + return pid_t(-1); +} ---------------- This should be out-of-lined. Furthermore, is there any place where the use requires process group handling? Otherwise, can't we just replace this with: ``` pid_t waitpid(pid_t pid, int *status, int options) { assert(pid > 0 && "only support waiting for a particular child"); assert(options ^ ~WNOHANG == 0 && "only WNOHANG is supported'); HANDLE hProcess; DWORD dwResult; hProcess = OpenProcess(SYNCHRONIZE, FALSE, pid); if (hProcess == NULL) return pid_t(-1); // TODO(compnerd) better error handling dwResult = WaitForSingleObject(hProcess, options & WNOHANG ? 0 : INFINITE); CloseHandle(hProcess); if (dwResult == WAIT_OBJECT_0 || dwResult == WAIT_TIMEOUT) return pid; return pid_t(-1); } ``` ================ Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:220 + // In most cases the missing notifications do not affect lldb-server + // so we are temporarily relaxing the following for Windows. +#if !defined(_WIN32) ---------------- amccarth wrote: > Hui wrote: > > amccarth wrote: > > > What's the scope of "temporarily"? Is there some specific feature or > > > change that will cause this workaround to be removed? > > Currently the absence of pty support is treated as a fatal error that will > > block the overall usage of lldb-server.exe for windows. In my opinion, the > > redirection of std -i/o/e are mainly intended to generate I* and O* packets > > to notify the lldb about the llgs's inferior (shown on lldb console). > > Without such support, some of the lldb-server functionalities, like > > launch/attach process and most of the remote packets etc. still can be > > tested or say be experimented (see the python tests under > > tools/lldb-server). > > > > Temporarily relax the mentioned codes and wait for the pty support on > > windows. No concrete idea how to add that support now. > Got it. I misunderstood the original comment and thought the "temporarily" > applied to the code change itself. PTY support is available as of 2019H1. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61686/new/ https://reviews.llvm.org/D61686 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits