[Lldb-commits] [PATCH] D89874: [lldb] Unify x86 watchpoint implementation on Linux and NetBSD (and future FreeBSD plugin) [WIP]

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.


================
Comment at: lldb/include/lldb/lldb-defines.h:57-58
 #define LLDB_WATCH_ID_IS_VALID(uid) ((uid) != (LLDB_INVALID_WATCH_ID))
 #define LLDB_WATCH_TYPE_READ (1u << 0)
 #define LLDB_WATCH_TYPE_WRITE (1u << 1)
 #define LLDB_WATCH_TYPE_IS_VALID(type)                                         \
----------------
I'm wondering if we could change these two values to avoid having separate `LLDB_GDB...`. I suppose it could break if API clients relied on them.


================
Comment at: lldb/source/Plugins/Process/Utility/WatchpointMixin_x86.cpp:59-60
+    if (error.Fail()) {
+      wp_index = LLDB_INVALID_INDEX32;
+      return error;
+    } else if (is_hit) {
----------------
Technically this could be replaced by `break`, and `return error` below but I suppose it's more readable as-is.


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

https://reviews.llvm.org/D89874