Re: [PATCH 50/51] core: changed retval of _cairo_composite_rectangles_init_for_mask() to cairo_bool_t

On Mon, Dec 21, 2015 at 05:47:26PM +0100, Enrico Weigelt, metux IT consult wrote:
> On 21.12.2015 13:41, Chris Wilson wrote:
> 
> Hi,
> 
> > Honestly, I prefer the interface to be consistent (i.e. all the
> > functions for the operations return the same type), consistent in error
> > propagation, and returning the status allows for more flexibilty in
> > future.
> 
> My intention is to limit the range to the actual domain.
> If we just use the 'big' status enum everywhere, we (or the compiler)
> don't really know which return values could happen.
> 
> > At the moment the callsites are essentially:
> > 
> > 	test; cjmp; undo stack; ret
> > 
> > afterwards they become;
> > 
> > 	test; cjmp; undo stack; mov NOTHING_TO_DO, eax; ret
> > 
> > or more likely;
> > 	test; cjmp -> ret
> > becomes
> > 	test; cjmp; mov NOTHING_TO_DO, eax; jmp -> ret
> 
> OTOH, the caller might be more complex - it needs to explicitly check
> for NOTHING_TO_DO. I'd suspect a check for zero/nonzero is a bit faster
> than test for specific value.
> 
> The current implementation explicitly assigns the return value to a
> local (likely on-stack) variable an then checks it. At least we should
> directly compare the it, w/o assignment, IMHO.

Which the compiler eats, it should not hit stack until later (or at all,
since it will be outside of its live range).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
-- 
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo