[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.

This looks good as long as SBMemoryRegionInfo and SBMemoryRegionInfoList are ready only and will never have any setter functions. If we plan to ever have the SBMemoryRegionInfo or SBMemoryRegionInfoList be modifiable, then we should make each contain a std::unique_ptr instead of a std::shared_ptr. Then the constructors always create an object and the copy constructor and assignment operators would just copy their contents from one to another instead of sharing the data. If you have code like:

  SBMemoryRegionInfo region;
  if (process.GetMemoryRegionInfo(0x123, region).Success())
  {
      SBMemoryRegionInfo region2(region);
      region2.SetSomething(xxxxx);
  }

Would you expect that the variable "region" now has the "SetSomething(xxxxx)" applied to it? I would expect not.

So I propose the following changes:

- make both SBMemoryRegionInfo and SBMemoryRegionInfoList contain std::unique_ptr objects
- have their constructors always create a default object
- change copy constructors to copy the objects instead of sharing a reference
- change assignment operators to copy the objects instead of sharing a reference


http://reviews.llvm.org/D20565