[cairo] [PATCH] Always call _cairo_region_get_boxes with an initialized 'boxes' pointer

Chris Wilson chris at chris-wilson.co.uk
Tue Dec 30 04:53:08 PST 2008


On Tue, 2008-12-30 at 19:09 -0800, Argiris Kirtzidis wrote:
> Hi all,
> 
> Code that calls _cairo_region_get_boxes will later call 
> _cairo_region_boxes_fini to free the 'boxes' pointer;
> here's a patch to initialize the pointer to NULL to avoid free'ing an 
> uninitialized pointer.

I'm intrigued as to what partial-analysis you performed that found these
issues - I'm always on the lookout for new bug hunting tools. From
reading the code, only the win32 usage might attempt to free an
uninitialised pointer. For clip and analysis-surface we should not even
call _cairo_region_get_boxes() unless we know that there is at least one
rectangle inside the region, which hopefully explains why we've never
managed to trigger this whilst valgrinding the test suite.

With regards to the patch, would it not be better to remove the
initialisation of nboxes in each of the call sequences and to ensure
that _cairo_region_get_boxes() always sets the out-parameters correctly
[on success]. Also whilst you are fixing _cairo_region_get_boxes(), you
should also protect the free() against NULL inside
_cairo_region_fini_boxes(), in order to be consistent across cairo in
defending ourselves against systems that disallow free(NULL).
-- 
Chris



More information about the cairo mailing list