labath added a comment.
first round of comments. I will give this another look tomorrow.
================
Comment at:
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64_not_crashed.cpp:7
+int
+bar(int x)
+{
----------------
Please format these consistently (llvm-style). clang-format will not work by
default as we are still ignoring the test folder.
================
Comment at: source/Plugins/Process/minidump/NtStructures.h:18
+// only about the position of the tls_slots.
+struct TEB64 {
+ llvm::support::ulittle64_t reserved1[12];
----------------
Please put this inside the lldb_private::minidump namespace.
================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:73
+
+ return lldb::ProcessSP(new ProcessMinidump(
+ target_sp, listener_sp, *crash_file, minidump_parser.getValue()));
----------------
return std::make_shared<ProcesMinidump>(...
================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:274
+ // 64 bit windows
+ std::string name_tolower = name.getValue();
+ std::transform(name_tolower.begin(), name_tolower.end(),
----------------
llvm::StringRef(*name).endswith_lower("wow64.dll")
================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:278
+ if (name_tolower.find("wow64.dll") != std::string::npos) {
+ m_is_wow64 = true;
+ }
----------------
There is a hidden assumption here that ReadModuleList() will be called before
any other function that depends on the value of `m_is_wow64`. (I am not even
sure if this is true in case of GetArchitecture().) I think we should make this
more robust. Maybe you could initialize this (along with the filtered_modules
list, I guess) in the constructor? It's not really doing extra work, as you're
going to need to parse the module list at some very early point anyway.
================
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:291
+ std::string module_names[16] = {
+ "D:"
+
"\\src\\llvm\\llvm\\tools\\lldb\\packages\\Python\\lldbsuite\\test\\funct"
----------------
Did clang-format break this in such an ugly way? What will happen if you use
raw strings instead: `R"(D:\no\double\back\slash)"` ?
https://reviews.llvm.org/D25905
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
