amccarth added a comment.
Nice!
================
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");
----------------
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.
================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:329
if (!module_sp || error.Fail()) {
- continue;
+ // We failed to locate a matching local object file. Fortunatelly
+ // the minidump format encodes enough information about each module's
----------------
nit: s/Fortunatelly/Fortunately/
================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:339
+ my_module->CreateImageSection(module, GetTarget());
+ module_sp = my_module;
+ GetTarget().GetImages().Append(module_sp);
----------------
If CreateImageSection bit were done by the PlaceholderModule's constructor,
then there'd be no need for the artificial `my_module` temporary, and nobody
would ever make a PlaceholderModule without an image section.
https://reviews.llvm.org/D45700
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
