Re: Fix some memory leaks + double free

On 09.05.2014 11:30, Sylvestre Ledru wrote:
> diff --git a/src/cairo-pdf-surface.c b/src/cairo-pdf-surface.c
> index c200c28..17fe1df 100644
> --- a/src/cairo-pdf-surface.c
> +++ b/src/cairo-pdf-surface.c
> @@ -2642,6 +2642,10 @@ _cairo_pdf_surface_lookup_jbig2_global (cairo_pdf_surface_t       *surface,
>      }
>  
>      global.id = malloc(global_id_length);
> +    if (unlikely (global.id == NULL)) {
> +	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
> +    }
> +
>      memcpy (global.id, global_id, global_id_length);
>      global.id_length = global_id_length;
>      global.res = _cairo_pdf_surface_new_object (surface);

Thanks, this is what I meant!

However, to fix your warnings from clang-analyze, don't you still need your
original change, too?

I mean this one:

diff --git a/src/cairo-pdf-surface.c b/src/cairo-pdf-surface.c
index c200c28..8e92df2 100644
--- a/src/cairo-pdf-surface.c
+++ b/src/cairo-pdf-surface.c
@@ -2645,8 +2645,10 @@ _cairo_pdf_surface_lookup_jbig2_global
(cairo_pdf_surface_t       *surface,
     memcpy (global.id, global_id, global_id_length);
     global.id_length = global_id_length;
     global.res = _cairo_pdf_surface_new_object (surface);
-    if (global.res.id == 0)
+    if (global.res.id == 0) {
+	free(global.id);
 	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+    }

     global.emitted = FALSE;
     status = _cairo_array_append (&surface->jbig2_global, &global);

So the first change fixes a NULL pointer dereference in cairo when malloc()
fails, the second change fixes a memory leak if _cairo_pdf_surface_new_object()
fails.

Uli
-- 
"Why make things difficult, when it is possible to make them cryptic
and totally illogical, with just a little bit more effort?" -- A. P. J.
-- 
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo