[Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added inline comments.


================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:172
+
+  // Do we support the minidump's architecture?
+  ArchSpec arch = GetArchitecture();
----------------
lemo wrote:
> amccarth wrote:
> > Should the architecture check be in the MinidumpParser::Initialize with the other checks?
> > 
> > I don't know the answer; I'm just asking for your thinking about this.
> Good question, here's my take: the checks are for consistency and a minidump with a currently unsupported architecture is a valid minidump. 
> 
> So I think it's better to have this check external since the architecture support is not a minidump parser concern. WDYT?
Yes, this makes sense to me.


https://reviews.llvm.org/D49202