[webkit-reviews] review granted: [Bug 20483] WebKit GTK Port needs a smartpointer to handle g_free (GFreePtr?) : [Attachment 24050] Add GOwnPtr

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Oct 3 12:53:27 PDT 2008

Darin Adler <darin at apple.com> has granted Marco Barisione
<marco.barisione at collabora.co.uk>'s request for review:
Bug 20483: WebKit GTK Port needs a smartpointer to handle g_free (GFreePtr?)

Attachment 24050: Add GOwnPtr

------- Additional Comments from Darin Adler <darin at apple.com>
Are you sure that overloading this for GDir and making it close is the right
thing to do? We don't do this for OwnPtr and FILE -- we try to limit ourselves
to things that are just simple freeing of memory.

 #include <wtf/Noncopyable.h>
+#include <wtf/GOwnPtr.h>

We normally leave a blank line to paragraph each #if section of includes

+    template <typename T> inline void freeOwnedPtr(T* ptr) {
g_free(reinterpret_cast<void*>(ptr)); }
+    template<> void freeOwnedPtr<GError>(GError*);
+    template<> void freeOwnedPtr<GList>(GList*);
+    template<> void freeOwnedPtr<GCond>(GCond*);
+    template<> void freeOwnedPtr<GMutex>(GMutex*);
+    template<> void freeOwnedPtr<GPatternSpec>(GPatternSpec*);
+    template<> void freeOwnedPtr<GDir>(GDir*);

The name for this needs to include "G" in it. We might later want to make one
that works with plain old free or with fastFree.

+	 T*& rawPtr() { return m_ptr; }

I'm sort of disappointed to see this. With this function, you can easily cause
a storage leak, overwriting something that's not freed. Maybe we should change
this to have an assertion that m_ptr is 0, and give it a name that makes it
clear that it's specifically for "out" arguments.

If this is something good, we probably want it in both GOwnPtr and OwnPtr, I

I'll say review+ even though I think those things should be resolved. The patch
is OK as-is, even though I think it could be improved.

More information about the webkit-reviews mailing list