[webkit-reviews] review denied: [Bug 21599] [GTK] WebKit GTK needs a wrapper for ref counted glib/gobject structs : [Attachment 43087] Updated patch which includes the adoptRef idiom

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Nov 25 22:35:25 PST 2009


Eric Seidel <eric at webkit.org> has denied Martin Robinson
<martin.james.robinson at gmail.com>'s request for review:
Bug 21599: [GTK] WebKit GTK needs a wrapper for ref counted glib/gobject
structs
https://bugs.webkit.org/show_bug.cgi?id=21599

Attachment 43087: Updated patch which includes the adoptRef idiom
https://bugs.webkit.org/attachment.cgi?id=43087&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Style:
1 template <> GtkTargetList* refGPtr(GtkTargetList* ptr);
 32 template <> void derefGPtr(GtkTargetList* ptr);
 33 
 34 template <> GdkCursor* refGPtr(GdkCursor* ptr);
 35 template <> void derefGPtr(GdkCursor* ptr);

(no arg names)

I wonder if we should write these with a PointerTraits helper type instead:
 31 template <> GtkTargetList* refGPtr(GtkTargetList* ptr);
 32 template <> void derefGPtr(GtkTargetList* ptr);

a stuct which had a ref and deref method, which one single generic refGPtr
could call instead.  Although I would be the first to tell you that I *am not*
a C++ template expert.
Maybe I'm also just not understanding what those are.

 167	 return GRefPtr<T>(p, true);
would be clearer using an Adopt enum instead of a boolean.
See examples like     enum PlacementNewAdoptType { PlacementNewAdopt }; in
RefPtr.h

Inconsistent spacing:
33 template<typename T> class GRefPtr;
 34 template <typename T> GRefPtr<T> adoptGRef(T*);

I think we need to go one more round on this.  r-


More information about the webkit-reviews mailing list