[Lldb-commits] [PATCH] D143215: Separate Process::GetWatchpointSupportInfo into two methods to get the two separate pieces of information

David Spickett via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Fri Feb 3 01:51:43 PST 2023


DavidSpickett added a reviewer: DavidSpickett.
DavidSpickett added a comment.

I like removing a return by ref and being explicit about what info is wanted where.

So say the stub reports the "watchpoints received" but not the number of slots.

Does lldb just send the watchpoint set packets and rely on the stub to error if it can't handle it (presumably you get a less than helpful error)? As opposed to if you had a number of slots lldb could track how many it had used and give you a nice error instead.



================
Comment at: lldb/include/lldb/Target/Process.h:1822-1825
+  /// One user specified watchpoint may require multiple hardware
+  /// watchpoints, e.g. a larger object, or an unaligned object.
+  /// A target stub may not allow the user to set that many distinct
+  /// watchpoints if one of these is true.
----------------
This is a bit muddled.

I guess your point is that number of watchpoints the user can set may be less than the number that the hardware supports. And that this method returns the latter not the former. So perhaps:
```
This number may be less than the number of watchpoints a user can specify. This is because a single user watchpoint may require multiple watchpoint slots to implement. Due to the size and/or alignment of objects.
```


================
Comment at: lldb/include/lldb/Target/Process.h:1830
+  ///     if unknown.
+  virtual uint32_t GetWatchpointSlotCount() {
+    return LLDB_INVALID_WATCHPOINT_SLOTS;
----------------
I would wave my `optional` flag here but defining some error value is the way of the land so I will keep it under wraps :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143215



More information about the lldb-commits mailing list