[Lldb-commits] [PATCH] D40311: elf-core: Split up parsing code into os-specific functions

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:488
+    ELFNote note = ELFNote();
+    note.Parse(segment, &offset);
+
----------------
Do we need to check anything after parsing a note here to ensure it parsed? Can offset end up not changing and could we get into an infinite loop here? Seems like we should do:
```
if (!note.Parse(segment, &offset))
  break;
```


================
Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:763-765
+    return llvm::make_error<llvm::StringError>(
+        "Don't know how to parse core file. Unsupported OS.",
+        llvm::inconvertibleErrorCode());
----------------
This won't cause a crash right?


================
Comment at: source/Plugins/Process/elf-core/ProcessElfCore.h:32
 #include "Plugins/ObjectFile/ELF/ELFHeader.h"
+#include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
+#include "Plugins/Process/elf-core/elf-core-enums.h"
----------------
Why is this needed here? Doesn't seem to be. Can you include only in .cpp file?


https://reviews.llvm.org/D40311