omjavaid added a comment.
Herald added a reviewer: JDevlieghere.

This seems fine to me with some minor nits. Also do you plan on writing a Linux 
API test for this which test memory regions on Linux? I couldnt locate one 
already written.



================
Comment at: lldb/source/API/SBMemoryRegionInfo.cpp:125
+
+bool SBMemoryRegionInfo::GetFlags(SBStream &flags) {
+  LLDB_RECORD_METHOD(bool, SBMemoryRegionInfo, GetFlags, (lldb::SBStream &),
----------------
This function always returns true. If there is no other use of HasFlags API 
function then may be merge GetFlags and HasFlags by returning false in case 
flags are not available.


================
Comment at: lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp:19
 
+enum MapKind { eMaps, eSMaps };
+
----------------
May be consider converting this into a class enum.


================
Comment at: lldb/unittests/Process/minidump/MinidumpParserTest.cpp:9
 
 #include "Plugins/Process/minidump/MinidumpParser.h"
 #include "Plugins/Process/minidump/MinidumpTypes.h"
----------------
This file apparently requires a clang-format run.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87442/new/

https://reviews.llvm.org/D87442

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to