labath added a comment.
+1 for tests. Other than that, the code seems fairly straight-forward, though
it could be brought closer to llvm style (e.g., by using more StringRefs in
favour of raw c strings).
================
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:709-714
+void MinidumpParser::ForEachStreamType(Callback const &callback) const {
+ for (auto const &pair: m_directory_map) {
+ if (!callback(pair.first, pair.second))
+ break;
+ }
+}
----------------
Why not just have an accessor which returns (a const reference to) the list of
streams. Then the users can iterate over this any way they see fit (and
range-based loops are easier to read than lambda callbacks).
================
Comment at: source/Plugins/Process/minidump/MinidumpParser.h:93
+ static const char *GetStreamTypeAsString(uint32_t stream_type);
+
----------------
Return llvm::StringRef here.
================
Comment at: source/Plugins/Process/minidump/MinidumpParser.h:96-99
+ const MinidumpLocationDescriptor &loc_desc)>
+ Callback;
+
+ void ForEachStreamType(Callback const &callback) const;
----------------
inconsistent spelling of const references (`const T &` vs `T const &`). I think
the first version is more prevalent in both lldb and llvm.
================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:561
+ ProcessMinidump *process =
+ (ProcessMinidump *)m_interpreter.GetExecutionContext().GetProcessPtr();
+ result.SetStatus(eReturnStatusSuccessFinishResult);
----------------
static_cast
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55727/new/
https://reviews.llvm.org/D55727
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
