[Lldb-commits] [PATCH] D23545: Minidump parsing

Zachary Turner via lldb-commits lldb-commits at lists.llvm.org
Fri Aug 19 14:19:37 PDT 2016

zturner added a comment.

Were you still going to change all the `Optionals` to raw pointers (or even better, `llvm::Errors`)

Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:21
@@ +20,3 @@
+MinidumpParser::MinidumpParser(const lldb::DataBufferSP &data_buf_sp)
+    : m_data_sp(data_buf_sp), m_header(nullptr), m_valid_data(true)
I'm not crazy about the idea of allowing the constructor to fail and then checking `IsValidData()` after you construct it.  One way LLVM solves this is by having a static function that returns `Expected<T>`.  You would use it like this:

Expected<MinidumpParser> MinidumpParser::create(const lldb::DataBufferSP &data_buf_sp) {
  if (data_buf_sp->GetByteSize() < sizeof(MinidumpHeader))
    return make_error<StringError>("Insufficient buffer!");

  MinidumpHeader *header = nullptr;
  if (auto EC = MinidumpHeader::Parse(header_data, header))
    return std::move(EC);


   return MinidumpParser(header, directory_map);

auto Parser = MinidumpParser::create(buffer);
if (!Parser) {
  // Handle the error

This way it's impossible to create a MinidumpParser that is invalid.

Up to you whether you want to do this, but I think it is a very good (and safe) pattern that I'd love to see LLDB start adopting more often.


More information about the lldb-commits mailing list