[cairo-commit] 15 commits - src/cairo-cff-subset.c src/cairo-clip.c src/cairo-ft-font.c src/cairo-hull.c src/cairo-image-surface.c src/cairoint.h src/cairo-pattern.c src/cairo-png.c src/cairo-truetype-subset.c src/cairo-type1-fallback.c src/cairo-win32-surface.c src/cairo-xlib-display.c src/cairo-xlib-surface.c test/a8-mask.c test/.gitignore test/Makefile.am test/png.c

Chris Wilson ickle at kemper.freedesktop.org
Wed Mar 5 06:38:20 PST 2008


 src/cairo-cff-subset.c      |   47 +++++++------
 src/cairo-clip.c            |    8 +-
 src/cairo-ft-font.c         |   12 ---
 src/cairo-hull.c            |   22 +++---
 src/cairo-image-surface.c   |   15 +++-
 src/cairo-pattern.c         |   25 ++-----
 src/cairo-png.c             |  121 ++++++++++++++++++++++++++---------
 src/cairo-truetype-subset.c |    7 --
 src/cairo-type1-fallback.c  |   10 +-
 src/cairo-win32-surface.c   |    2 
 src/cairo-xlib-display.c    |    1 
 src/cairo-xlib-surface.c    |   12 +--
 src/cairoint.h              |    1 
 test/.gitignore             |    1 
 test/Makefile.am            |    2 
 test/a8-mask.c              |   19 +++++
 test/png.c                  |  149 ++++++++++++++++++++++++++++++++++++++++++++
 17 files changed, 344 insertions(+), 110 deletions(-)

New commits:
commit 8ba8a1192497ff89215f8a1657cbe88609083fb1
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Tue Mar 4 13:59:57 2008 +0000

    [win32] make check doc fixup.
    
    Add the missing '%' to the macro as demanded by make check.

diff --git a/src/cairo-win32-surface.c b/src/cairo-win32-surface.c
index 4df953f..937b4e7 100644
--- a/src/cairo-win32-surface.c
+++ b/src/cairo-win32-surface.c
@@ -1678,7 +1678,7 @@ _cairo_win32_surface_show_glyphs (void			*surface,
  * Creates a cairo surface that targets the given DC.  The DC will be
  * queried for its initial clip extents, and this will be used as the
  * size of the cairo surface.  The resulting surface will always be of
- * format CAIRO_FORMAT_RGB24; should you need another surface format,
+ * format %CAIRO_FORMAT_RGB24; should you need another surface format,
  * you will need to create one through
  * cairo_win32_surface_create_with_dib().
  *
commit 1dd894115e03aa202941ecebe9fd3420c73645ef
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Tue Mar 4 09:04:17 2008 +0000

    [cairo-pattern] Raise an error for _cairo_pattern_create_solid() failure.
    
    Add a missing _cairo_error() for the malloc failure in
    _cairo_pattern_create_solid().

diff --git a/src/cairo-pattern.c b/src/cairo-pattern.c
index bcd8b2a..8ec3987 100644
--- a/src/cairo-pattern.c
+++ b/src/cairo-pattern.c
@@ -341,9 +341,10 @@ _cairo_pattern_create_solid (const cairo_color_t *color,
 	pattern = malloc (sizeof (cairo_solid_pattern_t));
     }
 
-    if (pattern == NULL)
+    if (pattern == NULL) {
+	_cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
 	pattern = (cairo_solid_pattern_t *) &_cairo_pattern_nil;
-    else
+    } else
 	_cairo_pattern_init_solid (pattern, color, content);
 
     return &pattern->base;
commit 5efc5238d548599a90a02d922d031a899424d1c1
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Feb 29 17:06:30 2008 +0000

    [cairo-hull] Propagate error during hull computation.
    
    Propagate the original error status instead of returning a new NO_MEMORY
    error.

diff --git a/src/cairo-hull.c b/src/cairo-hull.c
index d4e4af8..a9e55e6 100644
--- a/src/cairo-hull.c
+++ b/src/cairo-hull.c
@@ -44,8 +44,10 @@ typedef struct cairo_hull
     int id;
 } cairo_hull_t;
 
-static cairo_hull_t *
-_cairo_hull_create (cairo_pen_vertex_t *vertices, int num_vertices)
+static cairo_status_t
+_cairo_hull_create (cairo_pen_vertex_t	     *vertices,
+	            int			      num_vertices,
+		    cairo_hull_t	    **out)
 {
     int i;
     cairo_hull_t *hull;
@@ -63,10 +65,8 @@ _cairo_hull_create (cairo_pen_vertex_t *vertices, int num_vertices)
     vertices[0].point = tmp;
 
     hull = _cairo_malloc_ab (num_vertices, sizeof (cairo_hull_t));
-    if (hull == NULL) {
-	_cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
-	return NULL;
-    }
+    if (hull == NULL)
+	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
 
     for (i = 0; i < num_vertices; i++) {
 	hull[i].point = vertices[i].point;
@@ -83,7 +83,8 @@ _cairo_hull_create (cairo_pen_vertex_t *vertices, int num_vertices)
 	    hull[i].discard = 1;
     }
 
-    return hull;
+    *out = hull;
+    return CAIRO_STATUS_SUCCESS;
 }
 
 static int
@@ -190,12 +191,13 @@ _cairo_hull_to_pen (cairo_hull_t *hull, cairo_pen_vertex_t *vertices, int *num_v
 cairo_status_t
 _cairo_hull_compute (cairo_pen_vertex_t *vertices, int *num_vertices)
 {
+    cairo_status_t status;
     cairo_hull_t *hull;
     int num_hull = *num_vertices;
 
-    hull = _cairo_hull_create (vertices, num_hull);
-    if (hull == NULL)
-	return CAIRO_STATUS_NO_MEMORY;
+    status = _cairo_hull_create (vertices, num_hull, &hull);
+    if (status)
+	return status;
 
     qsort (hull + 1, num_hull - 1,
 	   sizeof (cairo_hull_t), _cairo_hull_vertex_compare);
commit 11a2444ec875aaaed12c1f1cfed5eb8e139c306d
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Jan 18 14:53:50 2008 +0000

    [cairo-png] Support generating CAIRO_FORMAT_RGB24 from PNGs.
    
    If the PNG does not have an alpha channel, then create an opaque image.

diff --git a/src/cairo-png.c b/src/cairo-png.c
index de09086..bdfeda2 100644
--- a/src/cairo-png.c
+++ b/src/cairo-png.c
@@ -33,6 +33,7 @@
  * Contributor(s):
  *	Carl D. Worth <cworth at cworth.org>
  *	Kristian Høgsberg <krh at redhat.com>
+ *	Chris Wilson <chris at chris-wilson.co.uk>
  */
 
 #include "cairoint.h"
@@ -304,7 +305,7 @@ cairo_surface_write_to_png (cairo_surface_t	*surface,
 }
 
 struct png_write_closure_t {
-    cairo_write_func_t	write_func;
+    cairo_write_func_t		 write_func;
     void			*closure;
 };
 
@@ -368,21 +369,21 @@ premultiply_data (png_structp   png,
     unsigned int i;
 
     for (i = 0; i < row_info->rowbytes; i += 4) {
-	uint8_t *base = &data[i];
+	uint8_t *base  = &data[i];
 	uint8_t  alpha = base[3];
 	uint32_t p;
 
 	if (alpha == 0) {
 	    p = 0;
 	} else {
-	    uint8_t  red = base[0];
+	    uint8_t  red   = base[0];
 	    uint8_t  green = base[1];
-	    uint8_t  blue = base[2];
+	    uint8_t  blue  = base[2];
 
 	    if (alpha != 0xff) {
-		red = multiply_alpha (alpha, red);
+		red   = multiply_alpha (alpha, red);
 		green = multiply_alpha (alpha, green);
-		blue = multiply_alpha (alpha, blue);
+		blue  = multiply_alpha (alpha, blue);
 	    }
 	    p = (alpha << 24) | (red << 16) | (green << 8) | (blue << 0);
 	}
@@ -390,6 +391,24 @@ premultiply_data (png_structp   png,
     }
 }
 
+/* Converts RGBx bytes to native endian xRGB */
+static void
+convert_bytes_to_data (png_structp png, png_row_infop row_info, png_bytep data)
+{
+    unsigned int i;
+
+    for (i = 0; i < row_info->rowbytes; i += 4) {
+	uint8_t *base  = &data[i];
+	uint8_t  red   = base[0];
+	uint8_t  green = base[1];
+	uint8_t  blue  = base[2];
+	uint32_t pixel;
+
+	pixel = (0xff << 24) | (red << 16) | (green << 8) | (blue << 0);
+	memcpy (base, &pixel, sizeof (uint32_t));
+    }
+}
+
 static cairo_surface_t *
 read_png (png_rw_ptr	read_func,
 	  void		*closure)
@@ -402,6 +421,7 @@ read_png (png_rw_ptr	read_func,
     png_uint_32 png_width, png_height;
     int depth, color_type, interlace, stride;
     unsigned int i;
+    cairo_format_t format;
     cairo_status_t status;
 
     /* XXX: Perhaps we'll want some other error handlers? */
@@ -445,7 +465,7 @@ read_png (png_rw_ptr	read_func,
         png_set_palette_to_rgb (png);
 
     /* expand gray bit depth if needed */
-    if (color_type == PNG_COLOR_TYPE_GRAY && depth < 8)
+    if (color_type == PNG_COLOR_TYPE_GRAY && (depth > 1 && depth < 8))
 #if PNG_LIBPNG_VER >= 10209
         png_set_expand_gray_1_2_4_to_8 (png);
 #else
@@ -462,20 +482,35 @@ read_png (png_rw_ptr	read_func,
         png_set_packing (png);
 
     /* convert grayscale to RGB */
-    if (color_type == PNG_COLOR_TYPE_GRAY
-        || color_type == PNG_COLOR_TYPE_GRAY_ALPHA)
-        png_set_gray_to_rgb (png);
+    if (color_type == PNG_COLOR_TYPE_GRAY ||
+	color_type == PNG_COLOR_TYPE_GRAY_ALPHA)
+    {
+	png_set_gray_to_rgb (png);
+    }
 
     if (interlace != PNG_INTERLACE_NONE)
         png_set_interlace_handling (png);
 
-    png_set_filler (png, 0xff, PNG_FILLER_AFTER);
+    switch (color_type) {
+	default:
+	case PNG_COLOR_TYPE_GRAY_ALPHA:
+	case PNG_COLOR_TYPE_RGB_ALPHA:
+	    format = CAIRO_FORMAT_ARGB32;
+	    png_set_read_user_transform_fn (png, premultiply_data);
+	    break;
 
-    png_set_read_user_transform_fn (png, premultiply_data);
+	case PNG_COLOR_TYPE_GRAY:
+	case PNG_COLOR_TYPE_PALETTE:
+	case PNG_COLOR_TYPE_RGB:
+	    format = CAIRO_FORMAT_RGB24;
+	    png_set_read_user_transform_fn (png, convert_bytes_to_data);
+	    png_set_filler (png, 0xff, PNG_FILLER_AFTER);
+	    break;
+    }
 
     png_read_update_info (png, info);
 
-    stride = cairo_format_stride_for_width (CAIRO_FORMAT_ARGB32, png_width);
+    stride = cairo_format_stride_for_width (format, png_width);
     if (stride < 0) {
 	surface = _cairo_surface_create_in_error (_cairo_error (CAIRO_STATUS_INVALID_STRIDE));
 	goto BAIL;
@@ -504,9 +539,9 @@ read_png (png_rw_ptr	read_func,
 	goto BAIL;
     }
 
-    surface = cairo_image_surface_create_for_data (data,
-						   CAIRO_FORMAT_ARGB32,
-						   png_width, png_height, stride);
+    surface = cairo_image_surface_create_for_data (data, format,
+						   png_width, png_height,
+						   stride);
     if (surface->status)
 	goto BAIL;
 
@@ -590,7 +625,7 @@ cairo_image_surface_create_from_png (const char *filename)
 }
 
 struct png_read_closure_t {
-    cairo_read_func_t	read_func;
+    cairo_read_func_t		 read_func;
     void			*closure;
 };
 
diff --git a/test/.gitignore b/test/.gitignore
index e56151d..0747521 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -123,6 +123,7 @@ pdf-features
 pdf-features.pdf
 pdf-surface-source
 pdf-surface-source.pdf
+png
 png-flatten
 ps-features
 ps-features.ps
diff --git a/test/Makefile.am b/test/Makefile.am
index 3020711..7201a98 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -96,6 +96,7 @@ paint-with-alpha$(EXEEXT)				\
 pattern-get-type$(EXEEXT)				\
 pattern-getters$(EXEEXT)				\
 pixman-rotate$(EXEEXT)					\
+png$(EXEEXT)						\
 push-group$(EXEEXT)					\
 radial-gradient$(EXEEXT)				\
 random-intersections$(EXEEXT)				\
@@ -623,6 +624,7 @@ fallback-resolution		\
 font-options			\
 multi-page			\
 pdf-features			\
+png				\
 ps-features			\
 svg-clip			\
 svg-surface			\
diff --git a/test/png.c b/test/png.c
new file mode 100644
index 0000000..65e465d
--- /dev/null
+++ b/test/png.c
@@ -0,0 +1,149 @@
+/*
+ * Copyright © 2008 Chris Wilson
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Chris Wilson not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior
+ * permission. Chris Wilson makes no representations about the
+ * suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * CHRIS WILSON DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS
+ * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS, IN NO EVENT SHALL CHRIS WILSON BE LIABLE FOR ANY SPECIAL,
+ * INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
+ * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR
+ * IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Author: Chris Wilson <chris at chris-wilson.co.uk>
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "cairo-test.h"
+
+#include <cairo.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <assert.h>
+
+/* Test the idempotency of write_png->read_png */
+
+#define RGB_MASK 0x00ffffff
+
+static cairo_bool_t
+image_surface_equals (cairo_surface_t *A, cairo_surface_t *B)
+{
+    if (cairo_image_surface_get_format (A) !=
+	cairo_image_surface_get_format (B))
+	return 0;
+
+    if (cairo_image_surface_get_width (A) !=
+	cairo_image_surface_get_width (B))
+	return 0;
+
+    if (cairo_image_surface_get_height (A) !=
+	cairo_image_surface_get_height (B))
+	return 0;
+
+    return 1;
+}
+
+static const char *
+format_to_string (cairo_format_t format)
+{
+    switch (format) {
+    case CAIRO_FORMAT_A1:     return "a1";
+    case CAIRO_FORMAT_A8:     return "a8";
+    case CAIRO_FORMAT_RGB24:  return "rgb24";
+    case CAIRO_FORMAT_ARGB32: return "argb32";
+    default: return "???";
+    }
+}
+
+static void
+print_surface (cairo_surface_t *surface)
+{
+    printf ("%s (%dx%d)\n",
+	    format_to_string (cairo_image_surface_get_format (surface)),
+	    cairo_image_surface_get_width (surface),
+	    cairo_image_surface_get_height (surface));
+}
+
+int
+main (void)
+{
+    cairo_surface_t *surface0, *surface1;
+    cairo_status_t status;
+    uint32_t argb32 = 0xdeadbede;
+
+    surface0 = cairo_image_surface_create_for_data ((unsigned char *) &argb32,
+						    CAIRO_FORMAT_ARGB32,
+						    1, 1, 4);
+    assert (cairo_surface_status (surface0) == CAIRO_STATUS_SUCCESS);
+    status = cairo_surface_write_to_png (surface0, "png-test.png");
+    if (status) {
+	printf ("Error writing 'png-test.png': %s\n",
+		cairo_status_to_string (status));
+	return CAIRO_TEST_FAILURE;
+    }
+    surface1 = cairo_image_surface_create_from_png ("png-test.png");
+    status = cairo_surface_status (surface1);
+    if (status) {
+	printf ("Error reading 'png-test.png': %s\n",
+		cairo_status_to_string (status));
+	return CAIRO_TEST_FAILURE;
+    }
+
+    if (! image_surface_equals (surface0, surface1)) {
+	printf ("Error surface mismatch.\n");
+	printf ("to png: "); print_surface (surface0);
+	printf ("from png: "); print_surface (surface1);
+	return CAIRO_TEST_FAILURE;
+    }
+    assert (*(uint32_t *) cairo_image_surface_get_data (surface1) == argb32);
+
+    cairo_surface_destroy (surface0);
+    cairo_surface_destroy (surface1);
+
+
+    surface0 = cairo_image_surface_create_for_data ((unsigned char *) &argb32,
+						    CAIRO_FORMAT_RGB24,
+						    1, 1, 4);
+    assert (cairo_surface_status (surface0) == CAIRO_STATUS_SUCCESS);
+    status = cairo_surface_write_to_png (surface0, "png-test.png");
+    if (status) {
+	printf ("Error writing 'png-test.png': %s\n",
+		cairo_status_to_string (status));
+	return CAIRO_TEST_FAILURE;
+    }
+    surface1 = cairo_image_surface_create_from_png ("png-test.png");
+    status = cairo_surface_status (surface1);
+    if (status) {
+	printf ("Error reading 'png-test.png': %s\n",
+		cairo_status_to_string (status));
+	return CAIRO_TEST_FAILURE;
+    }
+
+    if (! image_surface_equals (surface0, surface1)) {
+	printf ("Error surface mismatch.\n");
+	printf ("to png: "); print_surface (surface0);
+	printf ("from png: "); print_surface (surface1);
+	return CAIRO_TEST_FAILURE;
+    }
+    assert ((*(uint32_t *) cairo_image_surface_get_data (surface1) & RGB_MASK)
+	    == (argb32 & RGB_MASK));
+
+    cairo_surface_destroy (surface0);
+    cairo_surface_destroy (surface1);
+
+
+    return CAIRO_TEST_SUCCESS;
+}
commit 06b375aee999220ce294c22fa50a3040c19d5492
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Feb 29 11:55:01 2008 +0000

    [cairo-png] Use cairo_format_stride_for_width()
    
    Use cairo_format_stride_for_width() instead of assuming the pixel size
    and manually calculating the row stride. This should make it easier to
    support loading multiple image formats in future.

diff --git a/src/cairo-image-surface.c b/src/cairo-image-surface.c
index 8247e44..28e3ce2 100644
--- a/src/cairo-image-surface.c
+++ b/src/cairo-image-surface.c
@@ -440,6 +440,7 @@ cairo_format_stride_for_width (cairo_format_t	format,
 
     return ((bpp*width+7)/8 + STRIDE_ALIGNMENT-1) & ~(STRIDE_ALIGNMENT-1);
 }
+slim_hidden_def (cairo_format_stride_for_width);
 
 /**
  * cairo_image_surface_create_for_data:
diff --git a/src/cairo-png.c b/src/cairo-png.c
index 1030618..de09086 100644
--- a/src/cairo-png.c
+++ b/src/cairo-png.c
@@ -399,10 +399,9 @@ read_png (png_rw_ptr	read_func,
     png_info *info;
     png_byte *data = NULL;
     png_byte **row_pointers = NULL;
-    png_uint_32 png_width, png_height, stride;
-    int depth, color_type, interlace;
+    png_uint_32 png_width, png_height;
+    int depth, color_type, interlace, stride;
     unsigned int i;
-    unsigned int pixel_size;
     cairo_status_t status;
 
     /* XXX: Perhaps we'll want some other error handlers? */
@@ -476,8 +475,13 @@ read_png (png_rw_ptr	read_func,
 
     png_read_update_info (png, info);
 
-    pixel_size = 4;
-    data = _cairo_malloc_abc (png_height, png_width, pixel_size);
+    stride = cairo_format_stride_for_width (CAIRO_FORMAT_ARGB32, png_width);
+    if (stride < 0) {
+	surface = _cairo_surface_create_in_error (_cairo_error (CAIRO_STATUS_INVALID_STRIDE));
+	goto BAIL;
+    }
+
+    data = _cairo_malloc_ab (png_height, stride);
     if (data == NULL) {
 	surface = _cairo_surface_create_in_error (_cairo_error (CAIRO_STATUS_NO_MEMORY));
 	goto BAIL;
@@ -490,7 +494,7 @@ read_png (png_rw_ptr	read_func,
     }
 
     for (i = 0; i < png_height; i++)
-        row_pointers[i] = &data[i * png_width * pixel_size];
+        row_pointers[i] = &data[i * stride];
 
     png_read_image (png, row_pointers);
     png_read_end (png, info);
diff --git a/src/cairoint.h b/src/cairoint.h
index 773daf6..022033c 100644
--- a/src/cairoint.h
+++ b/src/cairoint.h
@@ -2244,6 +2244,7 @@ slim_hidden_proto (cairo_image_surface_create);
 slim_hidden_proto (cairo_image_surface_create_for_data);
 slim_hidden_proto (cairo_image_surface_get_height);
 slim_hidden_proto (cairo_image_surface_get_width);
+slim_hidden_proto (cairo_format_stride_for_width);
 slim_hidden_proto (cairo_line_to);
 slim_hidden_proto (cairo_mask);
 slim_hidden_proto (cairo_matrix_init);
commit b181f40949a855c957dc6e7a1033981a2ed7d05a
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Thu Feb 28 16:04:08 2008 +0000

    [test/a8-mask] Check negative strides as well.
    
    Check that we also allow surfaces to be created using a negative stride.

diff --git a/test/a8-mask.c b/test/a8-mask.c
index fec2fea..2e5efad 100644
--- a/test/a8-mask.c
+++ b/test/a8-mask.c
@@ -70,12 +70,16 @@ test_surface_with_width_and_stride (int width, int stride,
     cairo_test_status_t status;
     cairo_surface_t *surface;
     cairo_t *cr;
+    int len;
     unsigned char *data;
 
     cairo_test_log ("Creating surface with width %d and stride %d\n",
 		    width, stride);
 
-    data = xmalloc (stride);
+    len = stride;
+    if (len < 0)
+	len = -len;
+    data = xmalloc (len);
 
     surface = cairo_image_surface_create_for_data (data, CAIRO_FORMAT_A8,
 						   width, 1, stride);
@@ -123,6 +127,13 @@ draw (cairo_t *cr, int dst_width, int dst_height)
 	if (status)
 	    return status;
 
+	status = test_surface_with_width_and_stride (test_width,
+						     -test_width,
+						     expected);
+	if (status)
+	    return status;
+
+
 	/* Then create a surface using the correct stride, (should
 	   always succeed).*/
 	status = test_surface_with_width_and_stride (test_width,
@@ -130,6 +141,12 @@ draw (cairo_t *cr, int dst_width, int dst_height)
 						     CAIRO_STATUS_SUCCESS);
 	if (status)
 	    return status;
+
+	status = test_surface_with_width_and_stride (test_width,
+						     -stride,
+						     CAIRO_STATUS_SUCCESS);
+	if (status)
+	    return status;
     }
 
     /* Now test actually drawing through our mask data, allocating and
commit b6eb1c5c92321849661198facd53510366050d45
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Feb 29 11:49:14 2008 +0000

    [cairo-image-surface] Harden cairo_format_stride_for_width().
    
    Check the user supplied values for validity and potential overflow,
    returning -1 in such cases, and update the documentation to warn of the
    new error return.

diff --git a/src/cairo-image-surface.c b/src/cairo-image-surface.c
index 1704997..8247e44 100644
--- a/src/cairo-image-surface.c
+++ b/src/cairo-image-surface.c
@@ -418,7 +418,8 @@ _cairo_image_surface_create_with_content (cairo_content_t	content,
  * </programlisting></informalexample>
  *
  * Return value: the appropriate stride to use given the desired
- * format and width.
+ * format and width, or -1 if either the format is invalid or the width
+ * too large.
  *
  * Since: 1.6
  **/
@@ -426,7 +427,16 @@ int
 cairo_format_stride_for_width (cairo_format_t	format,
 			       int		width)
 {
-    int bpp = _cairo_format_bits_per_pixel (format);
+    int bpp;
+
+    if (! CAIRO_FORMAT_VALID (format)) {
+	_cairo_error_throw (CAIRO_STATUS_INVALID_FORMAT);
+	return -1;
+    }
+
+    bpp = _cairo_format_bits_per_pixel (format);
+    if ((unsigned) (width) >= (INT32_MAX - 7) / (unsigned) (bpp))
+	return -1;
 
     return ((bpp*width+7)/8 + STRIDE_ALIGNMENT-1) & ~(STRIDE_ALIGNMENT-1);
 }
commit c06d929325710c1a2cbecb8a64803ca8e1ffbec0
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Thu Feb 28 16:07:57 2008 +0000

    [cairo-png] Further hardening against malloc failures.
    
    On some OOM paths, libpng raises a warning as opposed to an error,
    these were not being propagated back to the caller. We were also not
    checking that we did not overwrite a pre-existing error status when
    raising an error whilst performing I/O.

diff --git a/src/cairo-png.c b/src/cairo-png.c
index 40cebe7..1030618 100644
--- a/src/cairo-png.c
+++ b/src/cairo-png.c
@@ -108,6 +108,13 @@ static void
 png_simple_warning_callback (png_structp png,
 	                     png_const_charp error_msg)
 {
+    cairo_status_t *error = png_get_error_ptr (png);
+
+    /* default to the most likely error */
+    if (*error == CAIRO_STATUS_SUCCESS)
+	*error = _cairo_error (CAIRO_STATUS_NO_MEMORY);
+
+    /* png does not expect to abort and will try to tidy up after a warning */
 }
 
 
@@ -138,8 +145,10 @@ write_png (cairo_surface_t	*surface,
         return status;
 
     /* PNG complains about "Image width or height is zero in IHDR" */
-    if (image->width == 0 || image->height == 0)
-	return _cairo_error (CAIRO_STATUS_WRITE_ERROR);
+    if (image->width == 0 || image->height == 0) {
+	status = _cairo_error (CAIRO_STATUS_WRITE_ERROR);
+	goto BAIL1;
+    }
 
     rows = _cairo_malloc_ab (image->height, sizeof (png_byte*));
     if (rows == NULL) {
@@ -222,8 +231,7 @@ write_png (cairo_surface_t	*surface,
     if (image->format == CAIRO_FORMAT_RGB24)
 	png_set_filler (png, 0, PNG_FILLER_AFTER);
 
-    if (rows)
-	png_write_image (png, rows);
+    png_write_image (png, rows);
     png_write_end (png, info);
 
 BAIL3:
@@ -248,7 +256,8 @@ stdio_write_func (png_structp png, png_bytep data, png_size_t size)
 	data += ret;
 	if (size && ferror (fp)) {
 	    cairo_status_t *error = png_get_error_ptr (png);
-	    *error = _cairo_error (CAIRO_STATUS_WRITE_ERROR);
+	    if (*error == CAIRO_STATUS_SUCCESS)
+		*error = _cairo_error (CAIRO_STATUS_WRITE_ERROR);
 	    png_error (png, NULL);
 	}
     }
@@ -309,7 +318,8 @@ stream_write_func (png_structp png, png_bytep data, png_size_t size)
     status = png_closure->write_func (png_closure->closure, data, size);
     if (status) {
 	cairo_status_t *error = png_get_error_ptr (png);
-	*error = status;
+	if (*error == CAIRO_STATUS_SUCCESS)
+	    *error = status;
 	png_error (png, NULL);
     }
 }
@@ -426,7 +436,10 @@ read_png (png_rw_ptr	read_func,
     png_get_IHDR (png, info,
                   &png_width, &png_height, &depth,
                   &color_type, &interlace, NULL, NULL);
-    stride = 4 * png_width;
+    if (status) { /* catch any early warnings */
+	surface = _cairo_surface_create_in_error (status);
+	goto BAIL;
+    }
 
     /* convert palette/gray image to rgb */
     if (color_type == PNG_COLOR_TYPE_PALETTE)
@@ -482,6 +495,11 @@ read_png (png_rw_ptr	read_func,
     png_read_image (png, row_pointers);
     png_read_end (png, info);
 
+    if (status) { /* catch any late warnings - probably hit an error already */
+	surface = _cairo_surface_create_in_error (status);
+	goto BAIL;
+    }
+
     surface = cairo_image_surface_create_for_data (data,
 						   CAIRO_FORMAT_ARGB32,
 						   png_width, png_height, stride);
@@ -514,7 +532,8 @@ stdio_read_func (png_structp png, png_bytep data, png_size_t size)
 	data += ret;
 	if (size && (feof (fp) || ferror (fp))) {
 	    cairo_status_t *error = png_get_error_ptr (png);
-	    *error = _cairo_error (CAIRO_STATUS_READ_ERROR);
+	    if (*error == CAIRO_STATUS_SUCCESS)
+		*error = _cairo_error (CAIRO_STATUS_READ_ERROR);
 	    png_error (png, NULL);
 	}
     }
@@ -581,7 +600,8 @@ stream_read_func (png_structp png, png_bytep data, png_size_t size)
     status = png_closure->read_func (png_closure->closure, data, size);
     if (status) {
 	cairo_status_t *error = png_get_error_ptr (png);
-	*error = status;
+	if (*error == CAIRO_STATUS_SUCCESS)
+	    *error = status;
 	png_error (png, NULL);
     }
 }
commit c985096e6d7c04a780c055387e17ec4bb9334db3
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Feb 29 11:16:39 2008 +0000

    [cairo-xlib] Tidy usage of _cairo_error().
    
    Avoid a duplicate call to _cairo_error() and add a missing one.

diff --git a/src/cairo-xlib-display.c b/src/cairo-xlib-display.c
index d10ed1e..20bad2e 100644
--- a/src/cairo-xlib-display.c
+++ b/src/cairo-xlib-display.c
@@ -268,6 +268,7 @@ _cairo_xlib_display_get (Display *dpy)
 
     codes = XAddExtension (dpy);
     if (codes == NULL) {
+	_cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
 	free (display);
 	display = NULL;
 	goto UNLOCK;
diff --git a/src/cairo-xlib-surface.c b/src/cairo-xlib-surface.c
index 3fcee05..59dd45b 100644
--- a/src/cairo-xlib-surface.c
+++ b/src/cairo-xlib-surface.c
@@ -466,6 +466,7 @@ _get_image_surface (cairo_xlib_surface_t    *surface,
     short x1, y1, x2, y2;
     cairo_format_masks_t masks;
     pixman_format_code_t pixman_format;
+    cairo_status_t status;
 
     x1 = 0;
     y1 = 0;
@@ -606,8 +607,11 @@ _get_image_surface (cairo_xlib_surface_t    *surface,
 							ximage->width,
 							ximage->height,
 							ximage->bytes_per_line);
-    if (image->base.status)
-	goto FAIL;
+    status = image->base.status;
+    if (status) {
+	XDestroyImage (ximage);
+	return status;
+    }
 
     /* Let the surface take ownership of the data */
     _cairo_image_surface_assume_ownership_of_data (image);
@@ -616,10 +620,6 @@ _get_image_surface (cairo_xlib_surface_t    *surface,
 
     *image_out = image;
     return CAIRO_STATUS_SUCCESS;
-
- FAIL:
-    XDestroyImage (ximage);
-    return _cairo_error (CAIRO_STATUS_NO_MEMORY);
 }
 
 static void
commit 914f4a3ec5a310c823558c27e500a23be808b9fe
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Feb 29 11:15:26 2008 +0000

    [cairo-pattern] Tidy usage of _cairo_error().
    
    Remove the duplicate calls to _cairo_error() along the constructor
    error paths and a missing call to _cairo_error().

diff --git a/src/cairo-pattern.c b/src/cairo-pattern.c
index 22a1a9e..bcd8b2a 100644
--- a/src/cairo-pattern.c
+++ b/src/cairo-pattern.c
@@ -406,7 +406,6 @@ _cairo_pattern_create_in_error (cairo_status_t status)
 cairo_pattern_t *
 cairo_pattern_create_rgb (double red, double green, double blue)
 {
-    cairo_pattern_t *pattern;
     cairo_color_t color;
 
     _cairo_restrict_value (&red,   0.0, 1.0);
@@ -417,12 +416,7 @@ cairo_pattern_create_rgb (double red, double green, double blue)
 
     CAIRO_MUTEX_INITIALIZE ();
 
-    pattern = _cairo_pattern_create_solid (&color,
-					   CAIRO_CONTENT_COLOR);
-    if (pattern->status)
-	_cairo_error_throw (pattern->status);
-
-    return pattern;
+    return _cairo_pattern_create_solid (&color, CAIRO_CONTENT_COLOR);
 }
 slim_hidden_def (cairo_pattern_create_rgb);
 
@@ -451,7 +445,6 @@ cairo_pattern_t *
 cairo_pattern_create_rgba (double red, double green, double blue,
 			   double alpha)
 {
-    cairo_pattern_t *pattern;
     cairo_color_t color;
 
     _cairo_restrict_value (&red,   0.0, 1.0);
@@ -463,12 +456,7 @@ cairo_pattern_create_rgba (double red, double green, double blue,
 
     CAIRO_MUTEX_INITIALIZE ();
 
-    pattern = _cairo_pattern_create_solid (&color,
-					   CAIRO_CONTENT_COLOR_ALPHA);
-    if (pattern->status)
-	_cairo_error_throw (pattern->status);
-
-    return pattern;
+    return _cairo_pattern_create_solid (&color, CAIRO_CONTENT_COLOR_ALPHA);
 }
 slim_hidden_def (cairo_pattern_create_rgba);
 
@@ -492,8 +480,10 @@ cairo_pattern_create_for_surface (cairo_surface_t *surface)
 {
     cairo_surface_pattern_t *pattern;
 
-    if (surface == NULL)
+    if (surface == NULL) {
+	_cairo_error_throw (CAIRO_STATUS_NULL_POINTER);
 	return (cairo_pattern_t*) &_cairo_pattern_nil_null_pointer;
+    }
 
     if (surface->status)
 	return (cairo_pattern_t*) _cairo_pattern_create_in_error (surface->status);
commit d9fb4d4bc55eae42f6348b142e667be454064e2c
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Feb 29 11:12:57 2008 +0000

    [cairo-ft-font] Simplify return of the nil font face during construction.
    
    Simply return the nil font face from the internal constructor rather
    than returning NULL and repeating the same fixup in the callers.

diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c
index 85ebce5..a781da4 100644
--- a/src/cairo-ft-font.c
+++ b/src/cairo-ft-font.c
@@ -2330,7 +2330,7 @@ _cairo_ft_font_face_create (cairo_ft_unscaled_font_t *unscaled,
     font_face = malloc (sizeof (cairo_ft_font_face_t));
     if (!font_face) {
 	_cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
-	return NULL;
+	return (cairo_font_face_t *)&_cairo_font_face_nil;
     }
 
     font_face->unscaled = unscaled;
@@ -2508,10 +2508,7 @@ cairo_ft_font_face_create_for_pattern (FcPattern *pattern)
     font_face = _cairo_ft_font_face_create (unscaled, &ft_options);
     _cairo_unscaled_font_destroy (&unscaled->base);
 
-    if (font_face)
-	return font_face;
-    else
-	return (cairo_font_face_t *)&_cairo_font_face_nil;
+    return font_face;
 }
 
 /**
@@ -2561,10 +2558,7 @@ cairo_ft_font_face_create_for_ft_face (FT_Face         face,
     font_face = _cairo_ft_font_face_create (unscaled, &ft_options);
     _cairo_unscaled_font_destroy (&unscaled->base);
 
-    if (font_face)
-	return font_face;
-    else
-	return (cairo_font_face_t *)&_cairo_font_face_nil;
+    return font_face;
 }
 
 /**
commit fd7d5d6e1dbad6c44a607d91ee59361ee19d32f9
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Feb 29 11:11:47 2008 +0000

    [cairo-clip] Raise _cairo_error() at original error site.
    
    In a couple of places where we detect an error and return an error
    object to the user, we did not throw a _cairo_error().

diff --git a/src/cairo-clip.c b/src/cairo-clip.c
index fd9f3a8..28057e5 100644
--- a/src/cairo-clip.c
+++ b/src/cairo-clip.c
@@ -744,8 +744,10 @@ _cairo_clip_copy_rectangle_list (cairo_clip_t *clip, cairo_gstate_t *gstate)
     if (clip->all_clipped)
 	goto DONE;
 
-    if (clip->path || clip->surface)
+    if (clip->path || clip->surface) {
+	_cairo_error_throw (CAIRO_STATUS_CLIP_NOT_REPRESENTABLE);
 	return (cairo_rectangle_list_t*) &_cairo_rectangles_not_representable;
+    }
 
     if (clip->has_region) {
 	cairo_box_int_t *boxes;
@@ -768,6 +770,7 @@ _cairo_clip_copy_rectangle_list (cairo_clip_t *clip, cairo_gstate_t *gstate)
 						    boxes[i].p2.y - boxes[i].p1.y };
 
 		if (!_cairo_clip_int_rect_to_user(gstate, &clip_rect, &rectangles[i])) {
+		    _cairo_error_throw (CAIRO_STATUS_CLIP_NOT_REPRESENTABLE);
 		    _cairo_region_boxes_fini (&clip->region, boxes);
 		    free (rectangles);
 		    return (cairo_rectangle_list_t*) &_cairo_rectangles_not_representable;
@@ -790,6 +793,7 @@ _cairo_clip_copy_rectangle_list (cairo_clip_t *clip, cairo_gstate_t *gstate)
 	if (_cairo_surface_get_extents (_cairo_gstate_get_target (gstate), &extents) ||
 	    !_cairo_clip_int_rect_to_user(gstate, &extents, rectangles))
 	{
+	    _cairo_error_throw (CAIRO_STATUS_CLIP_NOT_REPRESENTABLE);
 	    free (rectangles);
 	    return (cairo_rectangle_list_t*) &_cairo_rectangles_not_representable;
 	}
@@ -798,8 +802,8 @@ _cairo_clip_copy_rectangle_list (cairo_clip_t *clip, cairo_gstate_t *gstate)
  DONE:
     list = malloc (sizeof (cairo_rectangle_list_t));
     if (list == NULL) {
-        free (rectangles);
 	_cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
+        free (rectangles);
         return (cairo_rectangle_list_t*) &_cairo_rectangles_nil;
     }
 
commit 1654510a349d99167247d1004a481a95388cf0be
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Feb 29 11:10:34 2008 +0000

    [cairo-cff-subset] Propagate error status.
    
    Remove duplicate _cairo_error() by ensuring that the error status is
    always propagated from the original error site. Note that in one case
    this eliminates a potential _cairo_error(CAIRO_INT_STATUS_UNSUPPORTED)!

diff --git a/src/cairo-cff-subset.c b/src/cairo-cff-subset.c
index 344baf4..1c09492 100644
--- a/src/cairo-cff-subset.c
+++ b/src/cairo-cff-subset.c
@@ -444,31 +444,31 @@ _cairo_dict_init_key (cff_dict_operator_t *key, int operator)
     key->operator = operator;
 }
 
-static cff_dict_operator_t *
+static cairo_status_t
 cff_dict_create_operator (int            operator,
                           unsigned char *operand,
-                          int            operand_length)
+                          int            operand_length,
+			  cff_dict_operator_t **out)
 {
     cff_dict_operator_t *op;
 
     op = malloc (sizeof (cff_dict_operator_t));
-    if (op == NULL) {
-	_cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
-        return NULL;
-    }
+    if (op == NULL)
+	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
 
     _cairo_dict_init_key (op, operator);
     op->operand = malloc (operand_length);
     if (op->operand == NULL) {
         free (op);
-	_cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
-        return NULL;
+	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
     }
+
     memcpy (op->operand, operand, operand_length);
     op->operand_length = operand_length;
     op->operand_offset = -1;
 
-    return op;
+    *out = op;
+    return CAIRO_STATUS_SUCCESS;
 }
 
 static cairo_status_t
@@ -489,19 +489,21 @@ cff_dict_read (cairo_hash_table_t *dict, unsigned char *p, int dict_size)
             status = _cairo_array_append_multiple (&operands, p, size);
             if (status)
                 goto fail;
+
             p += size;
         } else {
             p = decode_operator (p, &operator);
-            op = cff_dict_create_operator (operator,
-                                           _cairo_array_index (&operands, 0),
-                                           _cairo_array_num_elements (&operands));
-            if (op == NULL) {
-                status = _cairo_error (CAIRO_STATUS_NO_MEMORY);
+            status = cff_dict_create_operator (operator,
+                                          _cairo_array_index (&operands, 0),
+                                          _cairo_array_num_elements (&operands),
+					  &op);
+            if (status)
                 goto fail;
-            }
+
             status = _cairo_hash_table_insert (dict, &op->base);
             if (status)
                 goto fail;
+
             _cairo_array_truncate (&operands, 0);
         }
     }
@@ -568,9 +570,9 @@ cff_dict_set_operands (cairo_hash_table_t *dict,
     }
     else
     {
-        op = cff_dict_create_operator (operator, operand, size);
-        if (op == NULL)
-	    return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+        status = cff_dict_create_operator (operator, operand, size, &op);
+        if (status)
+	    return status;
 
 	status = _cairo_hash_table_insert (dict, &op->base);
 	if (status)
@@ -838,6 +840,7 @@ cairo_cff_font_read_cid_fontdict (cairo_cff_font_t *font, unsigned char *ptr)
                                                    size);
         if (status)
             goto fail;
+
         /* Set integer operand to max value to use max size encoding to reserve
          * space for any value later */
         end_buf = encode_integer_max (buf, 0);
@@ -852,7 +855,7 @@ cairo_cff_font_read_cid_fontdict (cairo_cff_font_t *font, unsigned char *ptr)
 fail:
     cff_index_fini (&index);
 
-    return _cairo_error (status);
+    return status;
 }
 
 static cairo_int_status_t
@@ -1891,7 +1894,7 @@ fail2:
     free (font);
 fail1:
     free (name);
-    return _cairo_error (status);
+    return status;
 }
 
 static void
@@ -2011,7 +2014,7 @@ _cairo_cff_subset_init (cairo_cff_subset_t          *cff_subset,
  fail1:
     cairo_cff_font_destroy (font);
 
-    return _cairo_error (status);
+    return status;
 }
 
 void
@@ -2108,7 +2111,7 @@ fail2:
 fail1:
     _cairo_array_fini (&font->output);
     free (font);
-    return _cairo_error (status);
+    return status;
 }
 
 static cairo_int_status_t
commit 3b93d90edde7c065c6484e03e056b8605af85c4d
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Feb 29 10:16:05 2008 +0000

    [cairo-type1-fallback] Propagate original error status.
    
    _cairo_error() has already been called at the originating error site, so
    remove the duplicate (hard-coded!) call at the return statement.

diff --git a/src/cairo-type1-fallback.c b/src/cairo-type1-fallback.c
index 0ca4998..219bd39 100644
--- a/src/cairo-type1-fallback.c
+++ b/src/cairo-type1-fallback.c
@@ -78,6 +78,7 @@ cairo_type1_font_create (cairo_scaled_font_subset_t  *scaled_font_subset,
     cairo_matrix_t font_matrix;
     cairo_matrix_t ctm;
     cairo_font_options_t font_options;
+    cairo_status_t status;
 
     font = calloc (1, sizeof (cairo_type1_font_t));
     if (font == NULL)
@@ -106,7 +107,8 @@ cairo_type1_font_create (cairo_scaled_font_subset_t  *scaled_font_subset,
 							&font_matrix,
 							&ctm,
 							&font_options);
-    if (font->type1_scaled_font->status)
+    status = font->type1_scaled_font->status;
+    if (status)
         goto fail;
 
     _cairo_array_init (&font->contents, sizeof (unsigned char));
@@ -120,7 +122,7 @@ fail:
     free (font->widths);
     free (font);
 
-    return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+    return status;
 }
 
 /* Charstring commands. If the high byte is 0 the command is encoded
@@ -795,7 +797,7 @@ _cairo_type1_fallback_init_internal (cairo_type1_subset_t	*type1_subset,
     /* status is already set, ignore further errors */
     cairo_type1_font_destroy (font);
 
-    return _cairo_error (status);
+    return status;
 }
 
 cairo_status_t
@@ -886,7 +888,7 @@ fail2:
     _cairo_type2_charstrings_fini (type2_subset);
 fail1:
     cairo_type1_font_destroy (font);
-    return _cairo_error (status);
+    return status;
 }
 
 void
commit 141c54cd276ae86f6677fa8f66a118264d4287bf
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Feb 29 08:54:33 2008 +0000

    [cairo-truetype-subset] Remove duplicate _cairo_error().
    
    _cairo_error() has already been called at the originating error site, so
    remove the duplicate call at the return statement.

diff --git a/src/cairo-truetype-subset.c b/src/cairo-truetype-subset.c
index 557ab1d..6eeafa4 100644
--- a/src/cairo-truetype-subset.c
+++ b/src/cairo-truetype-subset.c
@@ -224,7 +224,7 @@ _cairo_truetype_font_create (cairo_scaled_font_subset_t  *scaled_font_subset,
         record = &(name->records[i]);
         if ((be16_to_cpu (record->platform) == 1) &&
             (be16_to_cpu (record->encoding) == 0) &&
-            (be16_to_cpu (record->name) == 4)) { 
+            (be16_to_cpu (record->name) == 4)) {
             font->base.base_font = malloc (be16_to_cpu(record->length) + 1);
             if (font->base.base_font) {
                 strncpy(font->base.base_font,
@@ -291,7 +291,7 @@ _cairo_truetype_font_create (cairo_scaled_font_subset_t  *scaled_font_subset,
     if (name)
 	free (name);
 
-    return _cairo_error (status);
+    return status;
 }
 
 static void
@@ -1121,9 +1121,6 @@ _cairo_truetype_subset_init (cairo_truetype_subset_t    *truetype_subset,
  fail1:
     cairo_truetype_font_destroy (font);
 
-    if (status != CAIRO_INT_STATUS_UNSUPPORTED)
-	status = _cairo_error (status);
-
     return status;
 }
 


More information about the cairo-commit mailing list