[Lldb-commits] [PATCH] D90102: [lldb] [Process/FreeBSDRemote] Enable watchpoint support

Tue Oct 27 03:28:33 PDT 2020

mgorny added inline comments.

================
Comment at: lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp:192-193
+    return Status("not implemented");
+  if (m_state == eStateLaunching)
+    return Status();
+  Status error = RemoveWatchpoint(addr);
----------------
labath wrote:
> mgorny wrote:
> > labath wrote:
> > > Are you actually using this state for anything? Should this be an assert? Maybe `assert(m_state == eStateStopped)` even ?
> > I don't know, I am a copy-paste monkey ;-). I've presumed LLDB core controls in which states the method will be called.
> Did you copy this code from the in-process plugin? That one uses more states (including this one), and I am not sure about the kind of controls it has.
> 
> lldb-server is much simpler, and and I'm confident that it won't call any of these functions when the process is not stopped (but asserting that doesn't hurt).. It also doesn't use eStateLaunching at all (unless you've added it somewhere in the freebsd code, but I hadn't noticed anything). This last bit is also the reason why I'd like to use a different State enum in lldb-server one day...
No, I've copied it from NetBSD. I suppose I'll end up backporting a lot of fixes to NetBSD plugin ;-).

So generally assert that it's stopped. Ok.

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

https://reviews.llvm.org/D90102