[cairo] Crash in cairo_surface_get_mime_data()

[Uli Schlachter]

On 04.10.2011 11:56, Chris Wilson wrote:
> On Mon, 03 Oct 2011 23:09:42 +0200, Uli Schlachter <psychon at znc.in> wrote:
> Non-text part: multipart/mixed
>> Hi,
>>
>> attached is a test case which crashes on every cairo version since 1.10. First
>> it sets some mime data, then it removes that mime data again. The next
>> cairo_surface_get_mime_data() then dereferences a NULL pointer.
>>
>> Also attached is a patch which fixes the issue for me.
>>
>> I wonder if this really is a cairo bug or if I'm just using the API in a wrong
>> way. Could someone enlighten me?
> 
> It's a bug. Should have realised when the docs said remove, it meant hide.
>  ;-)
> 
> Yours is a nice simple fix, though I wonder if we should fix
> _cairo_user_data_array_set_data(key, NULL) to actually remove the slot.

I think that could avoid a memory allocation later on. On the other hand. The
mime-data array is reinitialized all the time and removing and re-adding mime
data to a surface doesn't really make sense....

>> Then I also wonder how this should be added to the test suite. The only thing
>> that currently calls cairo_surface_get_mime_data() is api-special-cases, but
>> this problem doesn't really fit into that test. Should this get its own test
>> case, looking something like the code below?
> 
> Sure, start a suite of tests for mime-surface-api.c As we also want to make
> sure that we do get the exact same data returned from get as for set.
> (The drawing test should cover that, but doesn't actually imply no copy
> was made etc.)

Attached are two patches. First one adds a new test case and the other is the
one-line fix I posted earlier.

If no one complains, I'll eventually push this.

Cheers,
Uli
-- 
"Do you know that books smell like nutmeg or some spice from a foreign land?"
                                                  -- Faber in Fahrenheit 451