[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]

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?)
https://bugs.webkit.org/show_bug.cgi?id=20483

Attachment 24050: Add GOwnPtr
https://bugs.webkit.org/attachment.cgi?id=24050&action=edit

------- 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>
+#if PLATFORM(GTK)
+#include <wtf/GOwnPtr.h>
+#endif

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

+    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
think.

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.