labath added a comment.

The code seems reasonable, but I think it would be good include the set of 
tests which this patch is supposed to fix into the patch itself. (And same for 
other patches too.) There's also some functionality here that's probably not 
tested by any of our existing tests, so it would be good to have separate 
netbsd-specific tests too -- I'm mainly thinking of the code which mangles the 
resume actions into forms suitable for netbsd -- perhaps a test where one 
attempts to send two different signals to two threads (and gets an error in 
return), or when two (but not all) threads get the same signal.



================
Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:454-455
+    case eStateStopped:
+      if (action->signal != LLDB_INVALID_SIGNAL_NUMBER)
+        return Status("Passing signal to suspended thread unsupported");
+
----------------
Maybe this could be an assert ?


================
Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:477-481
+    signal = siginfo->psi_siginfo.si_signo;
+  }
+
+  ret = PtraceWrapper(PT_CONTINUE, GetID(), reinterpret_cast<void *>(1),
+                      signal);
----------------
(just curious) so you need to pass the signal both in the siginfo struct, and 
also to the PT_CONTINUE operation?


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

https://reviews.llvm.org/D70022



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

Reply via email to