[Lldb-commits] [PATCH] D62500: Add support to read aux vector values

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
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));
----------------
aadsm wrote:
> labath wrote:
> > 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.
> Can do, I was trying to follow this api though: http://man7.org/linux/man-pages/man3/getauxval.3.html.
> I think I'll go with the uint64_t though, I find it odd to return an addr_t because it's not an address, it's a number that happens to be the same size of an address.
Ah, interesting. It's a bit odd of an api as it does not allow you to distinguish AT_EUID not being present from somebody actually being root. I guess you're expected to assume that the AT_EUID field will always be present...

In any case, I think it would be better to use Optional, and not be tied to what is expressible in some C api.

I'm fine with uint64_t.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62500