Extend scoped_ptr to be closer to unique_ptr. Support custom deleters, and deleting arrays.

base/memory/scoped_ptr.h

Index: base/memory/scoped_ptr.h
diff --git a/base/memory/scoped_ptr.h b/base/memory/scoped_ptr.h
index 3547b7a15397f9c58f3c5193401d6d9defae333d..ed0e422af6c37ef9cffc068b27740482075e41cb 100644
--- a/base/memory/scoped_ptr.h
+++ b/base/memory/scoped_ptr.h
@@ -95,6 +95,8 @@
 #include <stddef.h>
 #include <stdlib.h>
 
+#include <algorithm>  // For std::swap().
+
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/move.h"
@@ -107,6 +109,47 @@ class RefCountedBase;
 class RefCountedThreadSafeBase;
 }  // namespace subtle
 
+// Function object which deletes its parameter, which must be a pointer.
+// If C is an array type, invokes 'delete[]' on the parameter; otherwise,
+// invokes 'delete'. The default deleter for scoped_ptr<T>.
+template <class T>
+struct DefaultDeleter {
+  DefaultDeleter() {}
+  template <typename U> DefaultDeleter(const DefaultDeleter<U>& other) {
+    // All default single-object deleters can trivially convert to one another.

[Ryan Sleevi, 2012/11/29 01:42:26]

20.7.1.1.2 2 has

"This constructor shall not participate in overload resolution unless U* is
implicitly convertable to T*."

I don't see anything in this class participating in that distinction.

I realize scoped_ptr_impl is probably guaranteeing this, by virtue of the
implicit conversion in scoped_ptr_impl::reset/scoped_ptr_impl::swap, so it's
probably a non-issue, but worth noting.

It seems like we can use an enable_if trick to ensure is_convertable, and then
let SFINAE remove this from the overload set if T/U are incomplete or
non-convertable types.

[gromer, 2012/12/04 19:41:18]

Oh, I see, this is an issue only because of the Chromium scoped_ptr's support
for move-emulation, so it doesn't apply to the Google-internal one. ajwong, this
one's in your court. FWIW I don't think SFINAE tricks are called for here, but a
comment might be appropriate.

More generally, it might make sense to explicitly document that DefaultDeleter
is an implementation detail of scoped_ptr, and should not be explicitly used by
client code.

[Ryan Sleevi, 2012/12/04 20:09:51]

gromer: I'm not sure I follow your remark. The spec comment is about the
constructor, but as implemented, the implicit type conversion isn't tested until
invoking operator()(T*), and finding that the U* originally used (for
DefaultDeleter<U>) doesn't allow conversion to T* - hence, operator()(U*) is
unresolved.

I agree that it's "mostly the same", but the note about 20.7.1.1.2.1 seems
applicable.

[gromer, 2012/12/04 20:39:27]

I'm afraid I don't follow you either; I was mostly agreeing with you that this
is "probably a non-issue, but worth noting". Let me try to rephrase:

First of all, this isn't really my department since it's not an issue for the
scoped_ptr that I maintain, so my comments should be considered advisory at
best.

That being said, my hunch is that it would be difficult to write client code
where the presence or absence of this SFINAE trick makes a visible difference,
and all but impossible to do so without making explicit mention of
DefaultDeleter. I'm open to refutation; what would be especially helpful is
example client code that is affected by the issue.

That being the case, my recommendation would to add a comment to discourage
independent use of DefaultDeleter, but not worry about the extra mess that
SFINAE tricks would entail. Remember, this is DefaultDeleter, not
std::default_delete; we only need to worry about inconsistency with the standard
to the extent that the difference can lead to actual bugs, and/or make it harder
to port scoped_ptr clients to unique_ptr in the future

[awong, 2012/12/12 02:17:03]

Reading through this, I think adding a COMPILE_ASSERT on on the result of
implicit_cast which should force the right typing constraints.

[gromer, 2012/12/12 17:29:57]

LG. 

+  }
+  inline void operator()(T* ptr) const {
+    enum { type_must_be_complete = sizeof(T) };
+    delete ptr;
+  }
+};
+
+// Specialization of DefaultDeleter for array types.
+template <class T>
+struct DefaultDeleter<T[]> {

[Ryan Sleevi, 2012/11/29 01:42:26]

You're missing the constructors here - in particular, the type conversion
handler.

[gromer, 2012/12/04 19:41:18]

"Missing" relative to what? std::default_delete<T[]> is specified to have only
one constructor, the 0-arg constructor, which is specified to use the default
implementation. That's exactly equivalent to what's provided here.

[Ryan Sleevi, 2012/12/04 20:09:51]

Sorry, ignore this. It's a disconnect between libc++, libstdc++, and MSVC (aka
dinkumware). I'd go with the spec here (20.7.1.1.3)

+  inline void operator()(T* ptr) const {
+    enum { type_must_be_complete = sizeof(T) };
+    delete[] ptr;
+  }
+
+ private:
+  // Disable this operator for any U != T because it is unsafe to execute
+  // an array delete when the static type of the array mismatches the dynamic
+  // type.
+  template <typename U> void operator()(U* array) const;
+};
+
+// Function object which invokes 'free' on its parameter, which must be
+// a pointer. Can be used to store malloc-allocated pointers in scoped_ptr:
+//
+// scoped_ptr<int, base::FreeDeleter> foo_ptr(
+//     static_cast<int*>(malloc(sizeof(int))));
+struct FreeDeleter {
+  inline void operator()(void* ptr) const {
+    free(ptr);
+  }
+};
+
 namespace internal {
 
 template <typename T> struct IsNotRefCounted {
@@ -117,7 +160,102 @@ template <typename T> struct IsNotRefCounted {
   };
 };
 
+// Minimal implementation of the core logic of scoped_ptr, suitable for
+// reuse in both scoped_ptr and its specializations.
+template <class T, class D>
+class scoped_ptr_impl {
+ public:
+  explicit scoped_ptr_impl(T* p) : data_(p) { }
+
+  // Initializer for deleters that have data parameters.
+  scoped_ptr_impl(T* p, const D& d) : data_(p, d) {}
+
+  // Templated constructor that destructively takes the value from another
+  // scoped_ptr_impl.
+  template <typename U, typename V>
+  scoped_ptr_impl(scoped_ptr_impl<U, V>* other)

[Jeffrey Yasskin, 2012/11/28 22:06:42]

Could you DISALLOW_COPY_AND_ASSIGN so there's no way to accidentally miss
calling this constructor?

[awong, 2012/11/29 01:15:13]

Done.

+      : data_(other->release(), other->get_deleter()) {
+    // We do not support move-only deleters.  We could modify our move
+    // emulation to have base::subtle::move() and base::subtle::forward()
+    // functions that are imperfect emulations of their C++11 equivalents,
+    // but until there's a requirement, just assume deleters are copyable.
+  }
+
+  template <typename U, typename V>
+  void TakeState(scoped_ptr_impl<U, V>* other) {
+    // See comment in templated constructor above regarding lack of support
+    // for move-only deleters.
+    reset(other->release());
+    get_deleter() = other->get_deleter();
+  }
+
+  ~scoped_ptr_impl() {
+    if (data_.ptr != NULL) {
+      // Not using get_deleter() saves one function call in non-optimized
+      // builds.
+      static_cast<D&>(data_)(data_.ptr);
+    }
+  }
+
+  void reset(T* p) {
+    // This self-reset check is deprecated.
+    // this->reset(this->get()) currently works, but it is DEPRECATED, and
+    // will be removed once we verify that no one depends on it.
+    //
+    // TODO(ajwong): Change this behavior to match unique_ptr<>.
+    // http://crbug.com/162971
+    if (p != data_.ptr) {
+      if (data_.ptr != NULL) {
+        // Note that this can lead to undefined behavior and memory leaks
+        // in the unlikely but possible case that get_deleter()(get())
+        // indirectly deletes this. The fix is to reset ptr_ before deleting
+        // its old value, but first we need to clean up the code that relies
+        // on the current sequencing.
+        static_cast<D&>(data_)(data_.ptr);
+      }
+      data_.ptr = p;
+    }
+  }
+
+  T* get() const { return data_.ptr; }
+
+  D& get_deleter() { return data_; }

[Ryan Sleevi, 2012/11/29 01:42:26]

Shouldn't you remove and then re-add the reference for D, in the event that D
was specified as a reference/const-reference? (unique_ptr permits this for
deleters)

[awong, 2012/12/12 02:17:03]

If I'm correct, right now we would just fail if D is a references type correct?

In that case, we're fulfilling the "strict unique_ptr<> subset" goal that
gromer@ was talking about. I'm okay with that since I don't think we have a
usecase needing a reference deleter type and fixing it requires making the
scoped_ptr(T*, d) constructor much more complicated.

[Ryan Sleevi, 2012/12/12 03:37:46]

Yup.

Being able to take pointer types "would be nice", but is not a must-have.

+  const D& get_deleter() const { return data_; }
+
+  void swap(scoped_ptr_impl& p2) {
+    // Standard swap idiom: 'using std::swap' ensures that std::swap is
+    // present in the overload set, but we call swap unqualified so that
+    // any more-specific overloads can be used, if available.
+    using std::swap;
+    swap(static_cast<D&>(data_), static_cast<D&>(p2.data_));
+    swap(data_.ptr, p2.data_.ptr);
+  }
+
+  T* release() {
+    T* old_ptr = data_.ptr;
+    data_.ptr = NULL;
+    return old_ptr;
+  }
+
+ private:
+  // Needed to allow type-converting constructor.
+  template <typename U, typename V> friend class scoped_ptr_impl;
+
+  // Use the empty base class optimization to allow us to have a D
+  // member, while avoiding any space overhead for it when D is an
+  // empty class.  See e.g. http://www.cantrip.org/emptyopt.html for a good
+  // discussion of this technique.
+  struct Data : public D {
+    explicit Data(T* ptr_in) : ptr(ptr_in) {}
+    Data(T* ptr_in, const D& other) : D(other), ptr(ptr_in) {}
+    T* ptr;
+  };
+
+  Data data_;
+};
+
 }  // namespace internal
+
 }  // namespace base
 
 // A scoped_ptr<T> is like a T*, except that the destructor of scoped_ptr<T>
@@ -129,93 +267,81 @@ template <typename T> struct IsNotRefCounted {
 //
 // The size of a scoped_ptr is small:
 // sizeof(scoped_ptr<C>) == sizeof(C*)
-template <class C>
+template <class T, class D = base::DefaultDeleter<T> >

[Ryan Sleevi, 2012/11/29 01:42:26]

20.7.1.2.1 requires that D be a function object type, lvalue-reference to
function, or lvalue-reference to a function object type.

Meaning, in particular, that D should not be a pointer type, AIUI. This is
enforced in libcxx, not sure about libstdc++.

[awong, 2012/12/12 02:17:03]

Right now, I think what happens is we don't support function pointers (and I'm
expilcitly ignoreing D& types).

We could support functions pointers by doing something like:

template <class T>
class scoped_ptr<T, void(T*)> { ... do something special ... };

but I want to hold off on this until we have a use case.

[Ryan Sleevi, 2012/12/12 03:37:46]

You could use a helper type to add a "deleter_type" typedef to the class.
If is_reference<D>, then deleter_type = typename remove_reference<D>.
If is_pointer<D>, then deleter_type == typename D.
Else, deleter_type = typename add_reference<D>

then have get_deleter() and friends return deleter_type

Maybe it's more complex than that though.

[awong, 2012/12/12 21:00:14]

Yeah, I think that would work. However, if I start handling reference types for
D, then the move constructor and move assignment get far far more complex.  I'm
happier having things just explode for now with a reference type :P

 class scoped_ptr {
   MOVE_ONLY_TYPE_FOR_CPP_03(scoped_ptr, RValue)
 
-  COMPILE_ASSERT(base::internal::IsNotRefCounted<C>::value,
-                 C_is_refcounted_type_and_needs_scoped_refptr);
+  COMPILE_ASSERT(base::internal::IsNotRefCounted<T>::value,
+                 T_is_refcounted_type_and_needs_scoped_refptr);
 
  public:
-
-  // The element type
-  typedef C element_type;
+  // The element and deleter types.

[Ryan Sleevi, 2012/11/29 01:42:26]

20.7.1.2.3 defines the ::pointer type, as according to

remove_reference<D>::type::pointer (if it exists), otherwise it's a synonym for
T*. This is due to the allocator_traits<> requirements (20.7.1.2.4)

Probably a non-issue, given that we don't use allocator_traits<> directly in
Chromium code, but worth mentioning about spec divergence.

[awong, 2012/12/12 02:17:03]

Good to know. Going to skip this one as well because it requires
remove_reference<> and I feel weird doing it partially here, but not for
Deleter.

+  typedef T element_type;
+  typedef D deleter_type;
 
   // Constructor.  Defaults to initializing with NULL.
-  // There is no way to create an uninitialized scoped_ptr.
-  // The input parameter must be allocated with new.
-  explicit scoped_ptr(C* p = NULL) : ptr_(p) { }
+  scoped_ptr() : impl_(NULL) { }
+
+  // Constructor.  Takes ownership of p.
+  explicit scoped_ptr(element_type* p) : impl_(p) { }
+
+  // Constructor.  Allows initialization of a stateful deleter.
+  scoped_ptr(element_type* p, const D& d) : impl_(p, d) { }
 
   // Constructor.  Allows construction from a scoped_ptr rvalue for a
-  // convertible type.
-  template <typename U>
-  scoped_ptr(scoped_ptr<U> other) : ptr_(other.release()) { }
+  // convertible type and deleter.
+  template <typename U, typename V>
+  scoped_ptr(scoped_ptr<U, V> other) : impl_(&other.impl_) { }

[Ryan Sleevi, 2012/11/29 01:42:26]

Note 20.7.1.2.1.19, which discards the move constructor from overload resolution
unless:
- D is a reference type and E (V) is the same type as D, or D is not a reference
type and E (V) is implicitly convertable to D.


In particular, and unless I'm missing something, this seems to allow you to have
a scoped_ptr<A[]> be released into a scoped_ptr<B>, assuming A and B are
convertable - which would be bad, right?

If not, why specialize scoped_ptr<A[]> at all (line 380), and not just instead
rely on the specialization of DefaultDeleter (line 129)?

[gromer, 2012/12/04 19:41:18]

As noted, the simulated move semantics are outside my expertise, but FWIW this
concern sounds plausible to me.
scoped_ptr<A[]> has a different method set than the base template; in
particular, it lacks operator* and operator->, and provides operator[] instead.

[awong, 2012/12/12 02:17:03]

Ick...nice catch.

Reading through 20.7.1.2.1.{15..17} and 20.7.1.2.1.{18..21} for
unique_ptr(unique_ptr&& u) and template <typename U, E>
unique_ptr(unique_ptr<U,E> && u), I think the distinction between the two
constructors only exist if

  (a) U is an array type
  (b) D is a reference type

If U is an array type, it shouldn't participate which will result in a compile
failure.  In our impl, a COMPILE_ASSERT should give equivalent safety.

If we disallow D being a reference type, then by spec we technically should
resolve to unique_ptr(unique_ptr&& u).  However, the only behavioral difference
is in the handling of D being a reference so I think we're okay not creating a
second constructor.

Does that sound right to peoples?

BTW, just tested passing from scoped_ptr<T[]> to scoped_ptr<T*>. Apparently we
were inadvertently saved by scoped_ptr<T[]> not friending the impl class.  Still
putting in the COMPILE_ASSERTs just to be explicit.

[Ryan Sleevi, 2012/12/12 03:37:46]

Sounds right to me.

[gromer, 2012/12/12 17:29:57]

LG.

 
   // Constructor.  Move constructor for C++03 move emulation of this type.
-  scoped_ptr(RValue rvalue)
-      : ptr_(rvalue.object->release()) {
-  }
-
-  // Destructor.  If there is a C object, delete it.
-  // We don't need to test ptr_ == NULL because C++ does that for us.
-  ~scoped_ptr() {
-    enum { type_must_be_complete = sizeof(C) };
-    delete ptr_;
-  }
+  scoped_ptr(RValue rvalue) : impl_(&rvalue.object->impl_) { }
 
   // operator=.  Allows assignment from a scoped_ptr rvalue for a convertible
-  // type.
-  template <typename U>
-  scoped_ptr& operator=(scoped_ptr<U> rhs) {
-    reset(rhs.release());
-    return *this;
-  }
-
-  // operator=.  Move operator= for C++03 move emulation of this type.
-  scoped_ptr& operator=(RValue rhs) {
-    swap(*rhs->object);
+  // type and deleter.
+  template <typename U, typename V>
+  scoped_ptr& operator=(scoped_ptr<U, V> rhs) {

[Ryan Sleevi, 2012/11/29 01:42:26]

same concerns here raised on 294

This is called out in 20.7.1.2.3.5, which requires that U not be an array type.

[awong, 2012/12/12 02:17:03]

See previous.

+    impl_.TakeState(&rhs.impl_);
     return *this;
   }
 
-  // Reset.  Deletes the current owned object, if any.
+  // Reset.  Deletes the currently owned object, if any.
   // Then takes ownership of a new object, if given.
-  // this->reset(this->get()) works.
-  void reset(C* p = NULL) {
-    if (p != ptr_) {
-      enum { type_must_be_complete = sizeof(C) };
-      delete ptr_;
-      ptr_ = p;
-    }
-  }
+  void reset(element_type* p = NULL) { impl_.reset(p); }
 
   // Accessors to get the owned object.
   // operator* and operator-> will assert() if there is no current object.
-  C& operator*() const {
-    assert(ptr_ != NULL);
-    return *ptr_;
+  element_type& operator*() const {
+    assert(impl_.get() != NULL);
+    return *impl_.get();
   }
-  C* operator->() const  {
-    assert(ptr_ != NULL);
-    return ptr_;
+  element_type* operator->() const  {
+    assert(impl_.get() != NULL);
+    return impl_.get();
   }
-  C* get() const { return ptr_; }
+  element_type* get() const { return impl_.get(); }
+
+  // Access to the deleter.
+  deleter_type& get_deleter() { return impl_.get_deleter(); }
+  const deleter_type& get_deleter() const { return impl_.get_deleter(); }
 
-  // Allow scoped_ptr<C> to be used in boolean expressions, but not
+  // Allow scoped_ptr<element_type> to be used in boolean expressions, but not
   // implicitly convertible to a real bool (which is dangerous).
-  typedef C* scoped_ptr::*Testable;
-  operator Testable() const { return ptr_ ? &scoped_ptr::ptr_ : NULL; }
+ private:
+  typedef base::internal::scoped_ptr_impl<element_type, deleter_type>
+      scoped_ptr::*Testable;
+
+ public:
+  operator Testable() const { return impl_.get() ? &scoped_ptr::impl_ : NULL; }
 
   // Comparison operators.
   // These return whether two scoped_ptr refer to the same object, not just to
   // two different but equal objects.
-  bool operator==(C* p) const { return ptr_ == p; }
-  bool operator!=(C* p) const { return ptr_ != p; }
+  bool operator==(element_type* p) const { return impl_.get() == p; }
+  bool operator!=(element_type* p) const { return impl_.get() != p; }
 
   // Swap two scoped pointers.
   void swap(scoped_ptr& p2) {
-    C* tmp = ptr_;
-    ptr_ = p2.ptr_;
-    p2.ptr_ = tmp;
+    impl_.swap(p2.impl_);
   }
 
   // Release a pointer.
@@ -223,44 +349,159 @@ class scoped_ptr {
   // If this object holds a NULL pointer, the return value is NULL.
   // After this operation, this object will hold a NULL pointer,
   // and will not own the object any more.
-  C* release() WARN_UNUSED_RESULT {
-    C* retVal = ptr_;
-    ptr_ = NULL;
-    return retVal;
+  element_type* release() WARN_UNUSED_RESULT {
+    return impl_.release();
   }
 
+  // C++98 doesn't support functions templates with default parameters which
+  // makes it hard to write a PassAs() that understands converting the deleter
+  // while preserving simple calling semantics.
+  //
+  // Until there is a use case for PassAs() with custom deleters, just ignore
+  // the custom deleter.
   template <typename PassAsType>
   scoped_ptr<PassAsType> PassAs() {
-    return scoped_ptr<PassAsType>(release());
+    return scoped_ptr<PassAsType>(Pass());
   }
 
  private:
-  C* ptr_;
+  // Needed to reach into |impl_| in the constructor.
+  template <typename U, typename V> friend class scoped_ptr;
+  base::internal::scoped_ptr_impl<element_type, deleter_type> impl_;
+
+  // Forbid comparison of scoped_ptr types.  If U != T, it totally
+  // doesn't make sense, and if U == T, it still doesn't make sense
+  // because you should never have the same object owned by two different
+  // scoped_ptrs.
+  template <class U> bool operator==(scoped_ptr<U> const& p2) const;
+  template <class U> bool operator!=(scoped_ptr<U> const& p2) const;
+};
+
+template <class T, class D>
+class scoped_ptr<T[], D> {
+  MOVE_ONLY_TYPE_FOR_CPP_03(scoped_ptr, RValue)
+
+ public:
+  // The element and deleter types.
+  typedef T element_type;
+  typedef D deleter_type;
+
+  // Constructor.  Defaults to initializing with NULL.
+  scoped_ptr() : impl_(NULL) { }
+
+  // Constructor. Stores the given array. Note that the argument's type
+  // must exactly match T*. In particular:
+  // - it cannot be a pointer to a type derived from T, because it is
+  //   inherently unsafe to access an array through a pointer whose
+  //   dynamic type does not match its static type. If you're doing this,
+  //   fix your code.
+  //   http://cplusplus.github.com/LWG/lwg-defects.html#938
+  // - it cannot be NULL, because NULL is an integral expression, not a
+  //   pointer to T. Use the no-argument version instead of explicitly
+  //   passing NULL.
+  // - it cannot be const-qualified differently from T per unique_ptr spec.

[Jeffrey Yasskin, 2012/11/28 22:06:42]

Perhaps mention that users should implicit_cast<const T*> to avoid this
restriction, but they should *not* implicit_cast<Base*> to get around the first
bullet.

[awong, 2012/11/29 01:15:13]

Done and also beefed up the first bullet.

+  //   http://cplusplus.github.com/LWG/lwg-active.html#2118
+  explicit scoped_ptr(element_type* array) : impl_(array) { }
+
+  // Constructor.  Move constructor for C++03 move emulation of this type.
+  scoped_ptr(RValue rvalue) : impl_(&rvalue.object->impl_) { }
 
-  // Forbid comparison of scoped_ptr types.  If C2 != C, it totally doesn't
-  // make sense, and if C2 == C, it still doesn't make sense because you should
-  // never have the same object owned by two different scoped_ptrs.
-  template <class C2> bool operator==(scoped_ptr<C2> const& p2) const;
-  template <class C2> bool operator!=(scoped_ptr<C2> const& p2) const;
+  // operator=.  Move operator= for C++03 move emulation of this type.
+  scoped_ptr& operator=(RValue rhs) {
+    impl_.TakeState(&rhs.object->impl_);
+    return *this;
+  }
+
+  // Reset.  Deletes the currently owned array, if any.
+  // Then takes ownership of a new object, if given.
+  void reset(element_type* array = NULL) { impl_.reset(array); }
+
+  // Accessors to get the owned array.
+  element_type& operator[](size_t i) const {
+    assert(impl_.get() != NULL);

[Ryan Sleevi, 2012/11/29 01:42:26]

I didn't think we "liked" using assert() - or at least, security team I seem to
recall had some sort of beef with it and ASAN.

I may be misremembering.

[awong, 2012/12/12 02:17:03]

I waffled on this one, but am leaving it as assert().  Rationale is all the
previous code in this file is also using assert().

+    return impl_.get()[i];
+  }
+  element_type* get() const { return impl_.get(); }
+
+  // Access to the deleter.
+  deleter_type& get_deleter() { return impl_.get_deleter(); }
+  const deleter_type& get_deleter() const { return impl_.get_deleter(); }
+
+  // Allow scoped_ptr<element_type> to be used in boolean expressions, but not
+  // implicitly convertible to a real bool (which is dangerous).
+ private:
+  typedef base::internal::scoped_ptr_impl<element_type, deleter_type>
+      scoped_ptr::*Testable;
 
+ public:
+  operator Testable() const { return impl_.get() ? &scoped_ptr::impl_ : NULL; }
+
+  // Comparison operators.
+  // These return whether two scoped_ptr refer to the same object, not just to
+  // two different but equal objects.
+  bool operator==(element_type* array) const { return impl_.get() == array; }
+  bool operator!=(element_type* array) const { return impl_.get() != array; }
+
+  // Swap two scoped pointers.
+  void swap(scoped_ptr& p2) {
+    impl_.swap(p2.impl_);
+  }
+
+  // Release a pointer.
+  // The return value is the current pointer held by this object.
+  // If this object holds a NULL pointer, the return value is NULL.
+  // After this operation, this object will hold a NULL pointer,
+  // and will not own the object any more.
+  element_type* release() WARN_UNUSED_RESULT {
+    return impl_.release();
+  }
+
+ private:
+  // Force element_type to be a complete type.
+  enum { type_must_be_complete = sizeof(element_type) };
+
+  // Actually hold the data.
+  base::internal::scoped_ptr_impl<element_type, deleter_type> impl_;
+
+  // Disable initialization from any type other than element_type*, by
+  // providing a constructor that matches such an initialization, but is
+  // private and has no definition. This is disabled because it is not safe to
+  // call delete[] on an array whose static type does not match its dynamic
+  // type.
+  template <typename U>
+  explicit scoped_ptr(U* array);
+
+  // Disable reset() from any type other than element_type*, for the same
+  // reasons as the constructor above.
+  template <typename U>
+  void reset(U* array);
+
+  // Forbid comparison of scoped_ptr types.  If U != T, it totally
+  // doesn't make sense, and if U == T, it still doesn't make sense
+  // because you should never have the same object owned by two different
+  // scoped_ptrs.
+  template <class U> bool operator==(scoped_ptr<U> const& p2) const;
+  template <class U> bool operator!=(scoped_ptr<U> const& p2) const;
 };
 
 // Free functions
-template <class C>
-void swap(scoped_ptr<C>& p1, scoped_ptr<C>& p2) {
+template <class T, class D>
+void swap(scoped_ptr<T, D>& p1, scoped_ptr<T, D>& p2) {
   p1.swap(p2);
 }
 
-template <class C>
-bool operator==(C* p1, const scoped_ptr<C>& p2) {
+template <class T, class D>
+bool operator==(T* p1, const scoped_ptr<T, D>& p2) {
   return p1 == p2.get();
 }
 
-template <class C>
-bool operator!=(C* p1, const scoped_ptr<C>& p2) {
+template <class T, class D>
+bool operator!=(T* p1, const scoped_ptr<T, D>& p2) {
   return p1 != p2.get();
 }
 
+// DEPRECATED: Use scoped_ptr<C[]> instead.
+//
 // scoped_array<C> is like scoped_ptr<C>, except that the caller must allocate
 // with new [] and the destructor deletes objects with delete [].
 //
@@ -298,7 +539,7 @@ class scoped_array {
 
   // operator=.  Move operator= for C++03 move emulation of this type.
   scoped_array& operator=(RValue rhs) {
-    swap(*rhs.object);
+    reset(rhs.object->release());
     return *this;
   }
 
@@ -380,19 +621,12 @@ bool operator!=(C* p1, const scoped_array<C>& p2) {
   return p1 != p2.get();
 }
 
-// This class wraps the c library function free() in a class that can be
-// passed as a template argument to scoped_ptr_malloc below.
-class ScopedPtrMallocFree {
- public:
-  inline void operator()(void* x) const {
-    free(x);
-  }
-};
-
+// DEPRECATED: Use scoped_ptr<C, base::FreeDeleter> instead.
+//
 // scoped_ptr_malloc<> is similar to scoped_ptr<>, but it accepts a
 // second template argument, the functor used to free the object.
 
-template<class C, class FreeProc = ScopedPtrMallocFree>
+template<class C, class FreeProc = base::FreeDeleter>
 class scoped_ptr_malloc {
   MOVE_ONLY_TYPE_FOR_CPP_03(scoped_ptr_malloc, RValue)
 
@@ -420,7 +654,7 @@ class scoped_ptr_malloc {
 
   // operator=.  Move operator= for C++03 move emulation of this type.
   scoped_ptr_malloc& operator=(RValue rhs) {
-    swap(*rhs.object);
+    reset(rhs.object->release());
     return *this;
   }
 
@@ -429,8 +663,10 @@ class scoped_ptr_malloc {
   // this->reset(this->get()) works.
   void reset(C* p = NULL) {
     if (ptr_ != p) {
-      FreeProc free_proc;
-      free_proc(ptr_);
+      if (ptr_ != NULL) {
+        FreeProc free_proc;
+        free_proc(ptr_);
+      }
       ptr_ = p;
     }
   }