[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).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64013/new/
https://reviews.llvm.org/D64013
More information about the lldb-commits
mailing list