mgorny added inline comments.

================
Comment at: lldb/include/lldb/Host/linux/Host.h:13
+#include "llvm/ADT/Optional.h"
+
+#include "lldb/lldb-types.h"
----------------
labath wrote:
> These spaces are pretty common in lldb, but that's not actually consistent 
> with how the rest of llvm formats them...
Will remove. However, note that this causes `clang-format` to reorder the 
includes.


================
Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:784
+    int status;
+    ::pid_t wait_pid = llvm::sys::RetryAfterSignal(-1, waitpid, -1, &status,
+                                                   WALLSIG | WNOHANG);
----------------
labath wrote:
> For NetBSD, it might be cleaner to keep waiting for the parent only, and then 
> do a separate waitpid for the child once you get its process id. That would 
> make the sequence of events (and the relevant code) predictable.
> 
> I think this is how this interface was meant to be used, even on linux, but 
> linux makes it tricky because it's not possible to wait for all threads 
> belonging to a given process in a single call -- one would have to iterate 
> through the threads and wait for each one separately (it might be interesting 
> to try to implement and benchmark that).
I've been thinking about it and I suppose it makes sense. I was considering 
what would be better to future-proof it for the future full multiprocess 
support but I suppose we could just loop over all process classes and let every 
one wait for its own process rather than creating a dedicated dispatcher that 
calls `waitpid(-1...)`.


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

https://reviews.llvm.org/D98822

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to