[Pixman] [PATCH 1/2] Always return a valid function from lookup_composite()

Chris Wilson chris at chris-wilson.co.uk
Mon Jan 14 11:29:02 PST 2013


On Mon, 14 Jan 2013 18:58:52 +0100, sandmann at cs.au.dk (=?utf-8?Q?S=C3=B8ren?= Sandmann) wrote:
> Hi Chris,
> 
> These patches generally make sense to me, but I have a few comments
> below.
> 
> > -    if (_pixman_implementation_lookup_composite (
> > -	    imp->toplevel, info->op,
> > -	    src_image->common.extended_format_code, src_flags,
> > -	    mask_format, mask_flags,
> > -	    dest_image->common.extended_format_code, info->dest_flags,
> > -	    &imp, &func))
> > +    _pixman_implementation_lookup_composite (
> > +	imp->toplevel, info->op,
> > +	src_image->common.extended_format_code, src_flags,
> > +	mask_format, mask_flags,
> > +	dest_image->common.extended_format_code, info->dest_flags,
> > +	&imp, &func);
> >      {
> 
> This looks like it leaves an isolated set of braces in the code.

I felt the existing block structure was useful for the local scoping, so
preserved it. Easy enough to move those to the top of the function and
let gcc do its thing.
 
> > diff --git a/pixman/pixman-glyph.c b/pixman/pixman-glyph.c
> > index 5451f42..5a271b6 100644
> > --- a/pixman/pixman-glyph.c
> > +++ b/pixman/pixman-glyph.c
> > @@ -464,13 +464,12 @@ pixman_composite_glyphs_no_mask (pixman_op_t            op,
> >  		    glyph_format = glyph_img->common.extended_format_code;
> >  		    glyph_flags = glyph_img->common.flags;
> >  
> > -		    if (! _pixman_implementation_lookup_composite (
> > -			    get_implementation(), op,
> > -			    src->common.extended_format_code, src->common.flags,
> > -			    glyph_format, glyph_flags | extra,
> > -			    dest_format, dest_flags,
> > -			    &implementation, &func))
> > -			goto out;
> 
> Was this patch generated as a diff against the first patch? I don't see
> "if (! _pixman_implementation_..." in the current code.

Right, this assumes the first patch to always use the return code as it
then makes sure we didn't miss one when converting the prototype.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


More information about the Pixman mailing list