[Lldb-commits] [PATCH] D149987: ObjectFile: introduce a COFF object file plugin

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.


================
Comment at: lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp:103-104
+
+  return new ObjectFileCOFF(module_sp, data_sp, data_offset, file, file_offset,
+                            length);
+}
----------------
compnerd wrote:
> bulbazord wrote:
> > Reading the implementation of the constructor, it looks like the constructor can fail to initialize correctly (specifically `m_object` may not be correctly populated). What are callers supposed to do in the way of validation here? Maybe there is further validation we can do in this function so that the constructor is only invoked if we're absolutely sure it will work?
> There isn't much you can do IMO.  The `new` can fail just as well - at which point, what do we do?  The constructor should only really fail if the return type from libLLVMObject has suddenly changed into an invalid type.  That cast really cannot fail in a way that we can recover from.
`createBinary` and the subsequent cast may not fail in a recoverable fashion but doing it in the constructor means that whatever is trying to create an object gets back an `ObjectFileCOFF` object even if it wasn't initialized correctly. If you did that work in `CreateInstance`, you could return `nullptr` if `createBinary` and that cast failed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149987/new/

https://reviews.llvm.org/D149987