[cairo] memory leak in _cairo_pattern_create_solid

[Chris Wilson]

Nguyen Vu Hung (vuhung16plus at gmail.com) said: 
> I think I've found a memory leak in cairo 1.4.10, file cairo-pattern.c,
> function _cairo_pattern_create_solid.
> In that function, variable *pattern is malloc()'ed but has not free() yet.
> 
> I am not so surely this is a bug because I am quite new to C and cairo.
> Please excuse me If I am wrong.

The 'leak' is intentional as we keep a few solid patterns around to avoid
having to continually allocate new patterns. In order to free up this
and other static caches cairo maintains, call
cairo_debug_reset_static_data() whilst exiting your code. You must be
sure that you have released all of your own cairo objects - contexts,
surfaces, patterns, fonts - before calling
cairo_debug_reset_static_data().
 
[snip]
 
> [vuhung@ cairo-1.4.10]$diff src/cairo-pattern.c src/cairo-
> pattern.c.2007-10-12.orig
> 317,321c317
> <     cairo_solid_pattern_t tmp_pattern;
> <       memcpy(&tmp_pattern, pattern, sizeof(pattern));
> <       if (pattern ) free(pattern);
> <     return &tmp_pattern.base;
> <
> ---
> >     return &pattern->base;

When diffing use 'diff -urNp original new', which is easier for humans
to read. Also note that here you are returning an address to a structure
stored on the stack (which promptly goes out of scope and will be
overwritten by the next function, hence the pattern itself will become
corrupted). By following the lifecycle of a pattern you will notice that
eventually it will be passed to free(), and since pattern is now a
pointer to a stack location the free() will go horribly wrong. And
calling free() there is dangerous because in the case of a malloc()
failure (when pattern ordinarily would be NULL) pattern becomes a
pointer to a 'nil object' (a statically allocated pattern with an error
status set) and so the if(pattern) will always be true, but you may
attempt to free() the statically allocated block. Besides if you are
concerned that pattern may be NULL, do not pass it to memcpy() without
checking! And the length of the memcpy is incorrect as
sizeof(pattern) == sizeof(void *) == 4 or 8 bytes, whereas
sizeof(tmp_pattern) == 132 bytes so the tmp_pattern itself is not fully
initialised.

Don't worry, the best way to learn C is by reading high quality code,
e.g. cairo, and experimenting. (Having K&R to hand helps ;-)

Have fun with Cairo!
--
Chris Wilson