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

[Argiris Kirtzidis]

Hi Chris,

Chris Wilson wrote:
> 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.

I found it because it was triggered by win32 usage as you correctly 
guessed. After fixing it for win32 I looked to other parts where it may 
cause the same issue.

>  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.
>   

I suspected that probably only win32 has a problem with this, but better 
be safe than sorry :-)

> 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].

After the f15b1f26becf28457e9ccf8903257a0dec25d547 commit ("[region] Use 
the caller supplied array for extracting boxes.") _cairo_region_get_boxes
could accept an array by the caller and initialisation of nboxes was 
added (but not for the array) at each call site. At this point was where 
the bug surfaced because _cairo_region_get_boxes was modified to *not* 
set the out-parameter array to NULL.

If _cairo_region_get_boxes should be modified again, can you be more 
specific about what this function should do ?

>  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).
>   

Ok, I'll address this in a follow-up patch.

-Argiris