[cairo] [PATCH] xcb: make use of _cairo_surface_is_xcb to check for surface type

[RAVI NANJUNDAPPA]

Hi, 

Please find attached the updated patch incorporated with review comments
from Uli. 
Thanking him for taking time to review the patch. 
Please help me in reviewing the attached patch.

Thanks and Best Regards, 
N Ravoi

> -----Original Message-----
> From: Uli Schlachter [mailto:psychon at znc.in]
> Sent: Monday, June 30, 2014 6:05 PM
> To: Ravi Nanjundappa; cairo at cairographics.org
> Subject: Re: [cairo] [PATCH] xcb: make use of _cairo_surface_is_xcb to
check
> for surface type
> 
> Hi,
> 
> On 30.06.2014 14:03, Ravi Nanjundappa wrote:
> > Introduced a new inline function _cairo_surface_is_xcb() as similar to
> > _cairo_surface_is_image() and used the same to check for xcb surface
> > type
> >
> > Signed-off-by: Ravi Nanjundappa <nravi.n at samsung.com>
> > ---
> >  src/cairo-xcb-surface.c |   20 ++++++++++++++++++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/cairo-xcb-surface.c b/src/cairo-xcb-surface.c index
> > c900edc..e3230c7 100644
> > --- a/src/cairo-xcb-surface.c
> > +++ b/src/cairo-xcb-surface.c
> > @@ -78,6 +78,22 @@ slim_hidden_proto
> (cairo_xcb_surface_create_with_xrender_format);
> >   * Since: 1.12
> >   **/
> >
> > +
> > +/**
> > + * _cairo_surface_is_xcb:
> > + * @surface: a #cairo_surface_t
> > + *
> > + * Checks if a surface is an #cairo_xcb_surface_t
> > + *
> > + * Return value: %TRUE if the surface is an xcb surface  **/ static
> > +inline cairo_bool_t _cairo_surface_is_xcb (const cairo_surface_t
> > +*surface) {
> > +    /* _cairo_surface_nil sets a NULL backend so be safe */
> > +    return surface->backend && surface->backend->type ==
> > +CAIRO_SURFACE_TYPE_XCB; }
> 
> 
> Does this new "->backend != NULL" check make a difference (= does this fix
> any
> bugs?) or is it just copied from _cairo_surface_is_image()?
> 
> >  cairo_surface_t *
> >  _cairo_xcb_surface_create_similar (void
> 	*abstract_other,
> >  				   cairo_content_t	 content,
> > @@ -1432,7 +1448,7 @@ cairo_xcb_surface_set_size (cairo_surface_t
> *abstract_surface,
> >      }
> >
> >
> > -    if (abstract_surface->type != CAIRO_SURFACE_TYPE_XCB) {
> > +    if (! _cairo_surface_is_xcb (abstract_surface)) {
> >  	_cairo_surface_set_error (abstract_surface,
> >  				  _cairo_error
> (CAIRO_STATUS_SURFACE_TYPE_MISMATCH));
> >  	return;
> > @@ -1486,7 +1502,7 @@ cairo_xcb_surface_set_drawable
> (cairo_surface_t *abstract_surface,
> >      }
> >
> >
> > -    if (abstract_surface->type != CAIRO_SURFACE_TYPE_XCB) {
> > +    if (! _cairo_surface_is_xcb (abstract_surface)) {
> >  	_cairo_surface_set_error (abstract_surface,
> >  				  _cairo_error
> (CAIRO_STATUS_SURFACE_TYPE_MISMATCH));
> >  	return;
> 
> What's special about these two places?
> 
> According to git grep, there are lots of other matches for
> CAIRO_SURFACE_TYPE_XCB, e.g. in cairo-xcb-surface-core.c or cairo-xcb-
> surface-render.c (and in cairo-xlib-xcb-surface.c, but I don't think that
this
> counts). Some of these check ->type, others do source->backend->type.
> 
> Since I don't have much time right now:
> 
> subsurfaces/cairo_surface_create_for_rectangle() creates a surface where -
> >type is the "original" type and ->backend->type is
> CAIRO_SURFACE_TYPE_SUBSURFACE, right? So we should always use -
> >backend->type internally so that subsurfaces can't mess things up, right?
(=
> This patch fixes an actual bug, but doesn't fix all the places where we
have
> this bug)
> 
> Sorry for this chaotic mail, I don't have much time right now.
> 
> Cheers,
> Uli
> --
> Sent from my Game Boy.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-xcb-make-use-of-_cairo_surface_is_xcb-to-check-for-s.patch
Type: application/octet-stream
Size: 3511 bytes
Desc: not available
URL: <http://lists.cairographics.org/archives/cairo/attachments/20140702/d031a4f6/attachment.obj>