[Lldb-commits] [PATCH] D64013: Correctly use GetLoadedModuleList to take advantage of libraries-svr4

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Jul 10 13:49:23 PDT 2019

labath added a comment.

Looks mostly good. Just a couple of style comments.

Comment at: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp:185-207
+  if (
+      // When the previous and current states are consistent this is the first
+      // time we have been asked to update.  Just take a snapshot of the
+      // currently loaded modules.
+      (m_previous.state == eConsistent && m_current.state == eConsistent) ||
+      // If we are about to add or remove a shared object clear out the current
+      // state and take a snapshot of the currently loaded images.
This logic is pretty hard to follow. I'd suggest trying to structure as some switch statements. Something like:
switch(current_state) {
case Consistent:
  switch (previous_state) {
    case Consistent: return TakeSnapshot;
    case Add: return Add;
    case Remove: return Remove;
case Add: case Remove:

Comment at: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp:240
+bool DYLDRendezvous::UpdateSOEntries() {
+  auto action = GetAction();
Use a switch here.

Comment at: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h:265
+  /// state
+  enum RendezvousAction GetAction() const;
We don't usually use the enum tag here.

Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:2731
   if (addr == LLDB_INVALID_ADDRESS) {
-    LoadedModuleInfoList list;
-    if (GetLoadedModuleList(list).Success())
-      addr = list.m_link_map;
+    llvm::Expected<LoadedModuleInfoList> list = GetLoadedModuleList();
+    if (list)
You'll need to somehow handle the case when GetLoadedModuleList fails (maybe by logging the error).

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list