[Lldb-commits] [PATCH] D32149: Correct handling NetBSD core(5) files with threads

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.


================
Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:302
+  // default 32 or 64 bit arch (without any architecture revision) based on
+  // object file's class.
   if (header.e_type == ET_CORE) {
----------------
joerg wrote:
> Unrelated cosmetic change.
I let clang-format to go and alter minor things. I can run clang-format over original files - commit, and add my diff again.


================
Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:463
 
-enum { NT_PROCINFO = 1, NT_AUXV, NT_AMD64_REGS = 33, NT_AMD64_FPREGS = 35 };
+enum { NT_PROCINFO = 1, NT_PROCINFO_SIZE = 160, NT_AUXV = 2 };
+
----------------
joerg wrote:
> Either sort them by value or by name, but not randomly
I will split this enum{} into two enums.


================
Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:523
+
+  offset += 108;
+  cpi_nlwps = data.GetU32(&offset); /* number of LWPs */
----------------
joerg wrote:
> Can you define a constant for the offset here below instead of a magic number?
I will try to get something to define aliases for these magic numbers.


================
Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:844
+  if (m_thread_data.size() != nlwps)
+    return Error("rror parsing NetBSD core(5) notes: Mismatch between the "
+                 "number of LWPs in netbsd_elfcore_procinfo and the number of "
----------------
joerg wrote:
> Typo
OK


================
Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:854
+  /* Signal destinated for a particular LWP */
+  else {
+    bool passed = false;
----------------
joerg wrote:
> Move the else to the } and the comment after the {
OK


Repository:
  rL LLVM

https://reviews.llvm.org/D32149