Merge "libutils: introduce sp<T>::make" (bdbd0dde) · Commits · e / os / android_system_core

libutils/StrongPointer_test.cpp

+3 −5

Original line number	Diff line number	Diff line
		@@ -36,10 +36,8 @@ private:

		TEST(StrongPointer, move) {
		bool isDeleted;
		SPFoo* foo = new SPFoo(&isDeleted);
		ASSERT_EQ(0, foo->getStrongCount());
		ASSERT_FALSE(isDeleted) << "Already deleted...?";
		sp<SPFoo> sp1(foo);
		sp<SPFoo> sp1 = sp<SPFoo>::make(&isDeleted);
		SPFoo* foo = sp1.get();
		ASSERT_EQ(1, foo->getStrongCount());
		{
		sp<SPFoo> sp2 = std::move(sp1);
		@@ -65,7 +63,7 @@ TEST(StrongPointer, NullptrComparison) {

		TEST(StrongPointer, PointerComparison) {
		bool isDeleted;
		sp<SPFoo> foo = new SPFoo(&isDeleted);
		sp<SPFoo> foo = sp<SPFoo>::make(&isDeleted);
		ASSERT_EQ(foo.get(), foo);
		ASSERT_EQ(foo, foo.get());
		ASSERT_NE(nullptr, foo);

libutils/include/utils/RefBase.h

+5 −0

Original line number	Diff line number	Diff line
		@@ -297,6 +297,11 @@ public:
		}

		protected:
		// When constructing these objects, prefer using sp::make<>. Using a RefBase
		// object on the stack or with other refcount mechanisms (e.g.
		// std::shared_ptr) is inherently wrong. RefBase types have an implicit
		// ownership model and cannot be safely used with other ownership models.

		RefBase();
		virtual ~RefBase();

libutils/include/utils/StrongPointer.h

+18 −3

Original line number	Diff line number	Diff line
		@@ -32,6 +32,12 @@ class sp {
		public:
		inline sp() : m_ptr(nullptr) { }

		// TODO: switch everyone to using this over new, and make RefBase operator
		// new private to that class so that we can avoid RefBase being used with
		// other memory management mechanisms.
		template <typename... Args>
		static inline sp<T> make(Args&&... args);

		sp(T* other); // NOLINT(implicit)
		sp(const sp<T>& other);
		sp(sp<T>&& other) noexcept;
		@@ -160,9 +166,6 @@ void sp_report_stack_pointer();
		// It does not appear safe to broaden this check to include adjacent pages; apparently this code
		// is used in environments where there may not be a guard page below (at higher addresses than)
		// the bottom of the stack.
		//
		// TODO: Consider adding make_sp<T>() to allocate an object and wrap the resulting pointer safely
		// without checking overhead.
		template <typename T>
		void sp<T>::check_not_on_stack(const void* ptr) {
		static constexpr int MIN_PAGE_SIZE = 0x1000; // 4K. Safer than including sys/user.h.
		@@ -174,6 +177,18 @@ void sp<T>::check_not_on_stack(const void* ptr) {
		}
		}

		// TODO: Ideally we should find a way to increment the reference count before running the
		// constructor, so that generating an sp<> to this in the constructor is no longer dangerous.
		template <typename T>
		template <typename... Args>
		sp<T> sp<T>::make(Args&&... args) {
		T* t = new T(std::forward<Args>(args)...);
		sp<T> result;
		result.m_ptr = t;
		t->incStrong(t); // bypass check_not_on_stack for heap allocation
		return result;
		}

		template<typename T>
		sp<T>::sp(T* other)
		: m_ptr(other) {