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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to