clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
So a bit more cleanup. We should always have a valid object inside of a
SBMemoryRegionInfo or SBMemoryRegionInfoList so there is no need to ever check
the m_opaque_ap to see if it contains a valid pointer so all if statements that
are doing:
if (m_opaque_ap.get())
m_opaque_ap->DoSomething();
Can be just:
m_opaque_ap->DoSomething();
Also, no functions should ever call "m_opaque_ap->reset();". All the changes
should be easy after that.
================
Comment at: source/API/SBMemoryRegionInfo.cpp:45-48
@@ +44,6 @@
+ {
+ if (rhs.IsValid())
+ ref() = rhs.ref();
+ else
+ m_opaque_ap.reset();
+ }
----------------
These 4 lines should just be:
```
ref() = rhs.ref();
```
We never want m_opaque_ap to contain an invalid pointer or we can crash.
================
Comment at: source/API/SBMemoryRegionInfo.cpp:60
@@ +59,3 @@
+{
+ return m_opaque_ap.get() != NULL;
+}
----------------
We shouldn't check for NULL since the auto pointer should never be invalid.
This should just call through and ask the underlying object:
```
return m_opaque_ap->IsValid();
```
If there is no IsValid() function on the underlying object, this function can
be removed.
================
Comment at: source/API/SBMemoryRegionInfo.cpp:66
@@ +65,3 @@
+{
+ m_opaque_ap.reset();
+}
----------------
We should never clear the underlying object, this should just call through:
```
m_opaque_ap->Clear();
```
================
Comment at: source/API/SBMemoryRegionInfo.cpp:72-74
@@ +71,5 @@
+{
+ if (m_opaque_ap)
+ return m_opaque_ap.get() == rhs.m_opaque_ap.get();
+ return false;
+}
----------------
This should compare the objects:
```
return ref() == rhs.ref();
```
================
Comment at: source/API/SBMemoryRegionInfo.cpp:80-82
@@ +79,5 @@
+{
+ if (m_opaque_ap)
+ return m_opaque_ap.get() != rhs.m_opaque_ap.get();
+ return false;
+}
----------------
This should compare the objects:
```
return ref() != rhs.ref();
```
================
Comment at: source/API/SBMemoryRegionInfo.cpp:88-89
@@ +87,4 @@
+{
+ if (m_opaque_ap.get() == NULL)
+ m_opaque_ap.reset (new MemoryRegionInfo ());
+ return *m_opaque_ap;
----------------
You should remove these lines now since m_opaque_ap will never contain an
invalid object.
================
Comment at: source/API/SBMemoryRegionInfo.cpp:101-104
@@ +100,6 @@
+SBMemoryRegionInfo::GetRegionBase () {
+ if (m_opaque_ap.get())
+ {
+ return m_opaque_ap->GetRange().GetRangeBase();
+ }
+ return 0;
----------------
No need to check the object
================
Comment at: source/API/SBMemoryRegionInfo.cpp:110-113
@@ +109,6 @@
+SBMemoryRegionInfo::GetRegionEnd () {
+ if (m_opaque_ap.get())
+ {
+ return m_opaque_ap->GetRange().GetRangeEnd();
+ }
+ return 0;
----------------
No need to check the object, just call the accessor
================
Comment at: source/API/SBMemoryRegionInfoList.cpp:108
@@ +107,3 @@
+{
+ if (m_opaque_ap.get())
+ return m_opaque_ap->GetSize();
----------------
No need to check the object, just call the accessor. Remove the if and the
other return.
================
Comment at: source/API/SBMemoryRegionInfoList.cpp:136
@@ +135,3 @@
+{
+ if (m_opaque_ap.get())
+ m_opaque_ap->Clear();
----------------
No need to check the object, just call the accessor. Remove the if statement.
================
Comment at: source/API/SBMemoryRegionInfoList.cpp:143
@@ +142,3 @@
+{
+ if (sb_region.IsValid() && m_opaque_ap.get())
+ m_opaque_ap->Append(sb_region);
----------------
No need to check m_opaque_ap since it will always be valid.
================
Comment at: source/API/SBMemoryRegionInfoList.cpp:150
@@ +149,3 @@
+{
+ if (sb_region_list.IsValid() && m_opaque_ap.get())
+ m_opaque_ap->Append(*sb_region_list);
----------------
No need to check m_opaque_ap since it will always be valid.
================
Comment at: source/API/SBMemoryRegionInfoList.cpp:157
@@ +156,3 @@
+{
+ return m_opaque_ap.get() != NULL;
+}
----------------
No need for an IsValid() since the list is always valid, this function can
actually be removed.
http://reviews.llvm.org/D20565
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
