[Lldb-commits] [PATCH] D20565: Add MemoryRegionInfo to SB API
Greg Clayton via lldb-commits
lldb-commits at lists.llvm.org
Fri May 27 09:58:59 PDT 2016
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:
if (process.GetMemoryRegionInfo(0x123, region).Success())
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
More information about the lldb-commits