[Webkit-unassigned] [Bug 144521] [X11] Add XUniquePtr and XUniqueResource to automatically free X resources

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=144521

--- Comment #10 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #9)
> Comment on attachment 252487 [details]
> Try to fix EFL build
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=252487&action=review

Thanks for the review.

> > Source/WebCore/PlatformEfl.cmake:200
> > +    platform/graphics/x11/XUnique.cpp
> 
> Consider splitting XUnique.h into XUniquePtr.h and XUniqueResource.h, and
> renaming XUnique.cpp to XUniqueResource.cpp.

Why? I think it's quite simple and reduces the amount of header includes.

> > Source/WebCore/platform/graphics/x11/XUnique.h:124
> > +    long unsigned release() { long unsigned resource = m_resource; m_resource = 0; return resource; }
> 
> Use std::exchange() here. (It's C++14, but there's an implementation in
> StdLibExtras.h.)

Cool.

> > Source/WebCore/platform/graphics/x11/XUnique.h:126
> > +    void reset(long unsigned resource = 0)
> 
> Instead of `resource.reset()`, I'd prefer resource = nullptr;
> If you consider it, it possible to implement by overloading the assignment
> operator:
>
>     XUniqueResource& operator=(std::nullptr_t) { ... }

But this resource is not a pointer. I added reset for consistency with std::unique_ptr, and didn't add the assign operator for nullptr, because it doesn't store a pointer.

> > Source/WebCore/platform/graphics/x11/XUnique.h:164
> > +typedef XUniqueResource<XResource::Colormap> XUniqueColormap;
> > +#if PLATFORM(GTK)
> > +typedef XUniqueResource<XResource::Damage> XUniqueDamage;
> > +#endif
> > +typedef XUniqueResource<XResource::Pixmap> XUniquePixmap;
> > +typedef XUniqueResource<XResource::Window> XUniqueWindow;
> > +#if USE(GLX)
> > +typedef XUniqueResource<XResource::GLXPbuffer> XUniqueGLXPbuffer;
> > +typedef XUniqueResource<XResource::GLXPixmap> XUniqueGLXPixmap;
> > +#endif
> > +
> > +// Give a name also to these XUniquePtr to avoid having to use the internal struct names (_XGC, __GLXcontextRec, ...).
> > +typedef XUniquePtr<_XGC> XUniqueGC;
> > +#if USE(GLX)
> > +typedef XUniquePtr<__GLXcontextRec> XUniqueGLXContext;
> > +#endif
> 
> Use type aliases:
> using XUniqueWhatever = XUniqueHandler<Whatever>;

Cool!

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150507/d934dbb0/attachment-0001.html>