[Lldb-commits] [PATCH] D61686: Enable lldb-server on Windows

Thu May 9 10:09:18 PDT 2019

compnerd added inline comments.

================
Comment at: include/lldb/Host/windows/PosixApi.h:106-109
+inline pid_t waitpid(pid_t pid, int *status, int options) {
+  // To be implemented.
+  return pid_t(-1);
+}
----------------
labath wrote:
> As discussed in the review where this was forked from, we shouldn't be mocking posix apis. Instead we should provide platform-independent abstractions that each platform can implement in it's own way. Waitpid is right now used in only one place. Implementing it here (even just as a stub) just encourages others to add more uses of it.
> 
> Given that (I think) you don't care about the "server" mode of lldb-platform at the moment, and the fact that the piece of code calling this is already broken because the use of fork, I think that for now we should just #ifdef-out the block of code calling fork and waitpid (it's already structured in a way that's easy to #ifdef. This would enable you to even provide a friendly error message explaining that the requested feature is not available.
I definitely am in favour of removal of the mocked POSIX API, to the point where I'm willing to entertain reduced functionality for that.  If we can completely preprocess away the usage instead of adding this, I am absolutely okay with that.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61686/new/

https://reviews.llvm.org/D61686