amccarth added a comment.


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.

lldb-commits mailing list

Reply via email to