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

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:488
+    ELFNote note = ELFNote();
+    note.Parse(segment, &offset);
+
----------------
clayborg wrote:
> 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;
> ```
Good point. There was no error checking in the original code, but it looks like there should be...


================
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());
----------------
clayborg wrote:
> This won't cause a crash right?
Only if the caller does not do anything with the result function (and it that case, it will assert regardless of whether the function returned an error or not).


https://reviews.llvm.org/D40311