[cairo] Clipping bogosity

[Behdad Esfahbod]

On Wed, 2008-04-16 at 10:30 -0400, Chris Wilson wrote:
> Adrian Johnson was trying to understand an apparent bug in
> _cairo_output_stream_vprintf():
> https://bugzilla.mozilla.org/show_bug.cgi?id=429071
> 
> Which appears quite bizarre, but it looks like it's just a bogus stack
> trace. Performing the same steps under valgrind offers a clue:
> ==28204== Conditional jump or move depends on uninitialised value(s)
> ==28204==    at 0x560587E: _cairo_surface_set_clip
> (cairo-surface.c:2016)
> ==28204==    by 0x561E489: _cairo_pdf_surface_emit_pattern
> (cairo-pdf-surface.c:1549)
> ==28204==    by 0x561FCEE: _cairo_pdf_surface_show_page
> (cairo-pdf-surface.c:3923)
> ==28204==    by 0x5605AAF: cairo_surface_show_page
> (cairo-surface.c:1749)
> ==28204==    by 0x5612442: _cairo_paginated_surface_show_page
> (cairo-paginated-surface.c:468)
> ==28204==    by 0x5605AAF: cairo_surface_show_page
> (cairo-surface.c:1749)
> ==28204==    by 0x51A78BC: gfxPDFSurface::EndPage()
> (gfxPDFSurface.cpp:94)
> 
> This appears to be due to the use of _cairo_surface_(get|set)_clip() to
> preserve the old clip around the replay of a meta-surface. As far as I
> can see, the current surface clip is modified in-place and always points
> at the clip embedded in the gstate. As such the attempt to save the clip
> around the call to replay the surface is futile. I've attached a patch
> to workaround the issue by perfoming a deep-copy of the clip onto the
> stack - but I think _cairo_surface_get_clip() is itself fundamentally
> flawed and a better API is required for preserving clips.

I'm not really qualified to comment on the meat of the patch, but I
don't like the repeated dupped code in error paths.  Just do the goto
thing...

-- 
behdad
http://behdad.org/

"Those who would give up Essential Liberty to purchase a little
 Temporary Safety, deserve neither Liberty nor Safety."
        -- Benjamin Franklin, 1759