[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