From 1d68548823d7b1e0bd186b924821cdbd80767be1 Mon Sep 17 00:00:00 2001
From: Steven Moreland <smoreland@google.com>
Date: Mon, 5 Apr 2021 20:36:34 +0000
Subject: [PATCH 1/2] libutils: add sp::cast method

Previously, sp::sp(T*) internally had a static cast, and people
frequently wrote code like this:

    sp<A> a = ...;
    sp<B> b(a.get()); // implicit static cast

Luckily, none of the other sp constructors have this implicit cast. So,
for explicit code, rather than making those use static_cast internally,
adding an sp::cast function.

Bug: 184190315
Test: use in libbinder
Change-Id: Id205c88d03e16cf85ccb8f493ce88b4bbc65a688
---
 libutils/include/utils/StrongPointer.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/libutils/include/utils/StrongPointer.h b/libutils/include/utils/StrongPointer.h
index 1f070524a..dd53b9ec0 100644
--- a/libutils/include/utils/StrongPointer.h
+++ b/libutils/include/utils/StrongPointer.h
@@ -72,6 +72,12 @@ public:
     template<typename U> sp(const sp<U>& other);  // NOLINT(implicit)
     template<typename U> sp(sp<U>&& other);  // NOLINT(implicit)
 
+    // Cast a strong pointer directly from one type to another. Constructors
+    // allow changing types, but only if they are pointer-compatible. This does
+    // a static_cast internally.
+    template <typename U>
+    static inline sp<T> cast(const sp<U>& other);
+
     ~sp();
 
     // Assignment
@@ -279,6 +285,12 @@ sp<T>::sp(sp<U>&& other)
     other.m_ptr = nullptr;
 }
 
+template <typename T>
+template <typename U>
+sp<T> sp<T>::cast(const sp<U>& other) {
+    return sp<T>::fromExisting(static_cast<T*>(other.get()));
+}
+
 template<typename T>
 sp<T>::~sp() {
     if (m_ptr)

From dc43fb279f6894383d3cee342085cde2869fcc7d Mon Sep 17 00:00:00 2001
From: Steven Moreland <smoreland@google.com>
Date: Wed, 7 Apr 2021 22:43:20 +0000
Subject: [PATCH 2/2] libutils: wp::fromExisting bugfix

This API was tested before, but it wasn't used until it is needed in
libbinder. Previously it passed the tests because wp::operator==
compares weak_ptrs which are never deleted, but it doesn't check the
value of m_ptr as well. This assumes that the RefBase implementation is
self-consistent.

Future considerations:
- add additional CHECK (perf?)
- add an additional optional CHECK?
- update all refbase tests to use an embellished form of this operator

Bug: 184190315
Test: libutils_test, boot and kill process when libbinder is using this
API

Change-Id: I66c97386d769529d5efae48e06775d4b4a344025
---
 libutils/RefBase_test.cpp        | 4 ++--
 libutils/include/utils/RefBase.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/libutils/RefBase_test.cpp b/libutils/RefBase_test.cpp
index dcc469e48..93f9654b1 100644
--- a/libutils/RefBase_test.cpp
+++ b/libutils/RefBase_test.cpp
@@ -242,12 +242,12 @@ TEST(RefBase, ReplacedComparison) {
 }
 
 TEST(RefBase, AssertWeakRefExistsSuccess) {
-    // uses some other refcounting method, or non at all
     bool isDeleted;
     sp<Foo> foo = sp<Foo>::make(&isDeleted);
     wp<Foo> weakFoo = foo;
 
     EXPECT_EQ(weakFoo, wp<Foo>::fromExisting(foo.get()));
+    EXPECT_EQ(weakFoo.unsafe_get(), wp<Foo>::fromExisting(foo.get()).unsafe_get());
 
     EXPECT_FALSE(isDeleted);
     foo = nullptr;
@@ -255,7 +255,7 @@ TEST(RefBase, AssertWeakRefExistsSuccess) {
 }
 
 TEST(RefBase, AssertWeakRefExistsDeath) {
-    // uses some other refcounting method, or non at all
+    // uses some other refcounting method, or none at all
     bool isDeleted;
     Foo* foo = new Foo(&isDeleted);
 
diff --git a/libutils/include/utils/RefBase.h b/libutils/include/utils/RefBase.h
index 5a5bd56e5..714894924 100644
--- a/libutils/include/utils/RefBase.h
+++ b/libutils/include/utils/RefBase.h
@@ -547,6 +547,7 @@ wp<T> wp<T>::fromExisting(T* other) {
     refs->incWeakRequireWeak(other);
 
     wp<T> ret;
+    ret.m_ptr = other;
     ret.m_refs = refs;
     return ret;
 }