[cairo] Consistent state during error recovery
Kristian Høgsberg
krh at bitplanet.net
Wed Feb 16 09:41:44 PST 2005
Mike Owens wrote:
> I believe I made an assumption about realloc's return value that doesn't hold
> true on all platforms, updated patch attached.
I've committed the memory leak stuffing parts of your patch (the
cairo_path.c and cairo_png_surface.c bits). However, I don't know that
we want to guarantee that the cairo_t is consistent and generally useful
in the face of CAIRO_STATUS_NO_MEMORY. It's a given that we shouldn't
leak in those cases and that a subsequent cairo_destroy() should
properly destroy the inconsistent cairo_t.
There is quite a few other entry points in the API that doesn't give you
this behavior, for example all the path construction operators can leave
the path inconsistent if they fail. This mean that you won't be able to
do fine grained OOOM handling by e.g. repeatedly setting the dash until
it succeeds. However, it will be possible to handle OOM at a higher
level, by e.g. creating a new cairo_t and retrying the entire drawing
sequence.
cheers,
Kristian
> ------------------------------------------------------------------------
>
> ? consistent_recovery.patch
> Index: src/cairo_gstate.c
> ===================================================================
> RCS file: /cvs/cairo/cairo/src/cairo_gstate.c,v
> retrieving revision 1.83
> diff -u -3 -p -r1.83 cairo_gstate.c
> --- src/cairo_gstate.c 13 Feb 2005 02:23:04 -0000 1.83
> +++ src/cairo_gstate.c 14 Feb 2005 17:38:40 -0000
> @@ -531,22 +531,21 @@ _cairo_gstate_current_line_join (cairo_g
> cairo_status_t
> _cairo_gstate_set_dash (cairo_gstate_t *gstate, double *dash, int num_dashes, double offset)
> {
> - if (gstate->dash) {
> - free (gstate->dash);
> - gstate->dash = NULL;
> - }
> -
> - gstate->num_dashes = num_dashes;
> - if (gstate->num_dashes) {
> - gstate->dash = malloc (gstate->num_dashes * sizeof (double));
> - if (gstate->dash == NULL) {
> - gstate->num_dashes = 0;
> - return CAIRO_STATUS_NO_MEMORY;
> + double *repl = gstate->dash;
> +
> + if(num_dashes != 0) {
> + repl = realloc (repl, (num_dashes * sizeof (double)));
> + if(repl == NULL)
> + return CAIRO_STATUS_NO_MEMORY;
> + } else {
> + free (repl);
> + repl = NULL;
> }
> - }
>
> - memcpy (gstate->dash, dash, gstate->num_dashes * sizeof (double));
> - gstate->dash_offset = offset;
> + gstate->dash = repl;
> + gstate->num_dashes = num_dashes;
> + memcpy (gstate->dash, dash, gstate->num_dashes * sizeof (double));
> + gstate->dash_offset = offset;
>
> return CAIRO_STATUS_SUCCESS;
> }
> Index: src/cairo_path.c
> ===================================================================
> RCS file: /cvs/cairo/cairo/src/cairo_path.c,v
> retrieving revision 1.17
> diff -u -3 -p -r1.17 cairo_path.c
> --- src/cairo_path.c 22 Oct 2004 01:40:50 -0000 1.17
> +++ src/cairo_path.c 14 Feb 2005 17:38:40 -0000
> @@ -100,7 +100,8 @@ _cairo_path_init_copy (cairo_path_t *pat
> for (other_op = other->op_head; other_op; other_op = other_op->next) {
> op = _cairo_path_op_buf_create ();
> if (op == NULL) {
> - return CAIRO_STATUS_NO_MEMORY;
> + _cairo_path_fini(path);
> + return CAIRO_STATUS_NO_MEMORY;
> }
> *op = *other_op;
> _cairo_path_add_op_buf (path, op);
> @@ -109,7 +110,8 @@ _cairo_path_init_copy (cairo_path_t *pat
> for (other_arg = other->arg_head; other_arg; other_arg = other_arg->next) {
> arg = _cairo_path_arg_buf_create ();
> if (arg == NULL) {
> - return CAIRO_STATUS_NO_MEMORY;
> + _cairo_path_fini(path);
> + return CAIRO_STATUS_NO_MEMORY;
> }
> *arg = *other_arg;
> _cairo_path_add_arg_buf (path, arg);
> Index: src/cairo_pattern.c
> ===================================================================
> RCS file: /cvs/cairo/cairo/src/cairo_pattern.c,v
> retrieving revision 1.18
> diff -u -3 -p -r1.18 cairo_pattern.c
> --- src/cairo_pattern.c 31 Jan 2005 16:50:22 -0000 1.18
> +++ src/cairo_pattern.c 14 Feb 2005 17:38:40 -0000
> @@ -223,14 +223,13 @@ cairo_pattern_add_color_stop (cairo_patt
> _cairo_restrict_value (&green, 0.0, 1.0);
> _cairo_restrict_value (&blue, 0.0, 1.0);
>
> - pattern->n_stops++;
> - pattern->stops = realloc (pattern->stops,
> - sizeof (cairo_color_stop_t) * pattern->n_stops);
> - if (pattern->stops == NULL) {
> - pattern->n_stops = 0;
> -
> - return CAIRO_STATUS_NO_MEMORY;
> - }
> + stop = realloc(pattern->stops,
> + sizeof (cairo_color_stop_t) * (pattern->n_stops + 1));
> + if (stop == NULL)
> + return CAIRO_STATUS_NO_MEMORY;
> +
> + pattern->stops = stop;
> + ++pattern->n_stops;
>
> stop = &pattern->stops[pattern->n_stops - 1];
>
> Index: src/cairo_png_surface.c
> ===================================================================
> RCS file: /cvs/cairo/cairo/src/cairo_png_surface.c,v
> retrieving revision 1.14
> diff -u -3 -p -r1.14 cairo_png_surface.c
> --- src/cairo_png_surface.c 31 Jan 2005 16:50:22 -0000 1.14
> +++ src/cairo_png_surface.c 14 Feb 2005 17:38:40 -0000
> @@ -331,13 +331,15 @@ _cairo_png_surface_copy_page (void *abst
> rows[i] = surface->image->data + i * surface->image->stride;
>
> png = png_create_write_struct (PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
> - if (png == NULL)
> - return CAIRO_STATUS_NO_MEMORY;
> + if (png == NULL) {
> + free(rows);
> + return CAIRO_STATUS_NO_MEMORY;
> + }
>
> info = png_create_info_struct (png);
> if (info == NULL) {
> - png_destroy_write_struct (&png, NULL);
> - return CAIRO_STATUS_NO_MEMORY;
> + status = CAIRO_STATUS_NO_MEMORY;
> + goto BAIL;
> }
>
> if (setjmp (png_jmpbuf (png))) {
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> cairo mailing list
> cairo at cairographics.org
> http://lists.freedesktop.org/mailman/listinfo/cairo
More information about the cairo
mailing list