[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

[Leonard Mosescu via Phabricator via lldb-commits]

lemo added inline comments.


================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:53
+  // Creates a synthetic module section covering the whole module image
+  void CreateImageSection(const MinidumpModule *module, Target& target) {
+    const ConstString section_name(".module_image");
----------------
amccarth wrote:
> I wonder if this should just be part of the constructor.  I don't see a scenario where you'd create a PlaceholderModule and not want to create exactly one fake image section.  I know the style guide is against doing lots of work in a constructor, but that's mostly because it's hard to report failures, which you're not doing here anyway.
Thanks for the suggestion. I agree, this would look a bit cleaner to fold everything in the constructor (except the extra arguments to the constructor, but it will probably still be a net win)

The reason for a separate method is the little "shared_from_this()". It can't be done from the constructor since the object is not yet managed by a shared_ptr, so we need to do it post-construction.


https://reviews.llvm.org/D45700