[Lldb-commits] [PATCH] D20565: Add MemoryRegionInfo to SB API

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Few minor changes, but looks great overall. Thanks for doing this.


================
Comment at: include/lldb/Target/MemoryRegionInfo.h:18-19
@@ -17,3 +17,4 @@
 {
-    class MemoryRegionInfo
+    class MemoryRegionInfo :
+            public std::enable_shared_from_this<MemoryRegionInfo>
     {
----------------
We don't need to inherit from std::enable_shared_from_this do we? Is there a future patch where you intend on using this? If not, lets leave it out for now and add it later if we need to.

================
Comment at: source/API/SBProcess.cpp:1482
@@ +1481,3 @@
+    ProcessSP process_sp(GetSP());
+    MemoryRegionInfoSP region_info = std::make_shared<lldb_private::MemoryRegionInfo>();
+    if (process_sp)
----------------
Please name "region_info_sp". All variables that are shared pointers should have "_sp" suffixes. 


http://reviews.llvm.org/D20565