labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
looks good, minor comments below.
================
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_32.h:37
+// This way we can reuse the already existing register contexts
+lldb::DataBufferSP ConvertMinidumpContext_x86_32_WithRegIface(
+ llvm::ArrayRef<uint8_t> source_data,
----------------
You don't need the `WithRegIface` suffix. That's an ObjC style, which we don't
use in LLVM
================
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_32.h:52
+ enum {
+ RegisteAreaSize = 80,
+ };
----------------
typo
================
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_32.h:115
+
+// For context_flags. These values indicate the type of
+// context stored in the structure. The high 24 bits identify the CPU, the
----------------
This comment doesn't seem to apply to the macro below it.
================
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:279
// Register stuff
// TODO probably split register stuff tests into different file?
+#define REG_VAL32(x) *(reinterpret_cast<uint32_t *>(x))
----------------
If you want to split them off to a different file, do it now. If not, remove
the todo. (I don't see a reason for the split btw)
https://reviews.llvm.org/D25832
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
