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

Reply via email to