labath added inline comments.
================
Comment at:
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:91-94
+ DataExtractor auxv_data(m_process ? m_process->GetAuxvData() :
DataBufferSP(),
+ m_process->GetByteOrder(),
+ m_process->GetAddressByteSize());
+ m_auxv.reset(new AuxVector(auxv_data));
----------------
It looks like we're not guarding the call to `LoadModules` on line 101, so I
think we can assume that m_process is not null here (and I don't see why it
should be -- it does not make sense to call `DidAttach` if there is no process
around).
BTW, your guarding attempt isn't even correct as we'd always call
`m_process->GetByteOrder()` and crash if the process was null.
================
Comment at:
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:182-185
+ DataExtractor auxv_data(m_process ? m_process->GetAuxvData() :
DataBufferSP(),
+ m_process->GetByteOrder(),
+ m_process->GetAddressByteSize());
+ m_auxv.reset(new AuxVector(auxv_data));
----------------
same here.
================
Comment at:
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:549-550
- if (ModuleSP module_sp = target.GetOrCreateModule(module_spec,
- true /* notify */)) {
+ if (ModuleSP module_sp =
+ target.GetOrCreateModule(module_spec, true /* notify */)) {
UpdateLoadedSections(module_sp, LLDB_INVALID_ADDRESS, m_interpreter_base,
----------------
Whitespace-only change. Did you maybe run clang-format on the whole file
instead of just the diff?
It doesn't matter that much here as there isn't many of them, but I just want
to make sure you're aware of this in the future.
================
Comment at: lldb/source/Plugins/Process/Utility/AuxVector.cpp:38
-AuxVector::AuxVector(Process *process) : m_process(process) {
- DataExtractor data;
- Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
-
- data.SetData(GetAuxvData());
- data.SetByteOrder(m_process->GetByteOrder());
- data.SetAddressByteSize(m_process->GetAddressByteSize());
-
- ParseAuxv(data);
-
- if (log)
- DumpToLog(log);
-}
-
-AuxVector::iterator AuxVector::FindEntry(EntryType type) const {
- for (iterator I = begin(); I != end(); ++I) {
- if (I->type == static_cast<uint64_t>(type))
- return I;
- }
-
- return end();
+uint64_t AuxVector::GetAuxValue(enum EntryType entry_type) const {
+ auto it = m_auxv_entries.find(static_cast<uint64_t>(entry_type));
----------------
It would be better to return an Optional<uint64_t> (or addr_t maybe ?) instead
of using a magic value to mean "not found". It looks like at least some of
these entries can validly be zero.
================
Comment at: lldb/source/Plugins/Process/Utility/AuxVector.cpp:50
log->PutCString("AuxVector: ");
- for (iterator I = begin(); I != end(); ++I) {
- log->Printf(" %s [%" PRIu64 "]: %" PRIx64, GetEntryName(*I), I->type,
- I->value);
+ for (auto it = m_auxv_entries.begin(); it != m_auxv_entries.end(); ++it) {
+ log->Printf(" %s [%" PRIu64 "]: %" PRIx64,
----------------
range-based for ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62500/new/
https://reviews.llvm.org/D62500
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
