[Lldb-commits] [PATCH] D24385: MinidumpParsing: pid, modules, exceptions

[Dimitar Vlahovski via lldb-commits]

dvlahovski added inline comments.

================
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:222
@@ -154,1 +221,2 @@
+  return MinidumpExceptionStream::Parse(data);
 }
----------------
Now that I return an ArrayRef<ModuleList>, which is basically a reinterpret cast of a piece of memory to a list of modules, do you think that this is still something good to do?

At least for now, I'm calling this method only once in the DoLoadCore initializing method of the plugin (still unsubmitted code) and then creating the ModuleSpecs for each module.

================
Comment at: source/Plugins/Process/minidump/MinidumpParser.h:46
@@ -45,2 +45,3 @@
 
-  llvm::Optional<std::vector<const MinidumpThread *>> GetThreads();
+  llvm::Optional<std::string> GetMinidumpString(uint32_t rva);
+
----------------
Specifically for the string - I think it's best to return llvm::Optional<std::string> in order to distinguish between empty string and error. And there are some conditions that need to be met for the string to parsed correctly.

================
Comment at: source/Plugins/Process/minidump/MinidumpParser.h:60
@@ +59,3 @@
+
+  llvm::ArrayRef<MinidumpModule> GetModuleList();
+
----------------
I am returning an ArrayRef<MinidumpModule>. The Modules are consecutive in memory so this seemed like a good idea

================
Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:21
@@ +20,3 @@
+llvm::StringRef
+lldb_private::minidump::consumeString(llvm::ArrayRef<uint8_t> &Buffer) {
+  auto str = llvm::StringRef(reinterpret_cast<const char *>(Buffer.data()),
----------------
So, breakpad specific string in the Minidump files are not UTF-16 string, but just ascii strings in the file (not null terminated).
When I get the stream "LinuxProcStatus" I know how big it is and set it in the ArrayRef.
So basically it is my intention here to parse the entire ArrayRef to a string


https://reviews.llvm.org/D24385