[Lldb-commits] [PATCH] D87442: [lldb] Show flags for memory regions

[Muhammad Omair Javaid via Phabricator via lldb-commits]

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