[cairo-commit] src/cairo-base85-stream.c src/cairoint.h src/cairo-output-stream.c src/cairo-pdf-surface.c src/cairo-ps-surface.c src/cairo-svg-surface.c

Carl Worth cworth at kemper.freedesktop.org
Tue Apr 4 10:48:03 PDT 2006


 src/cairo-base85-stream.c |   40 ++++++----------------
 src/cairo-output-stream.c |   82 ++++++++++++++++++++++++++++++----------------
 src/cairo-pdf-surface.c   |   12 ++++--
 src/cairo-ps-surface.c    |   27 +++++++--------
 src/cairo-svg-surface.c   |   16 ++++++--
 src/cairoint.h            |   35 +++++++++++++++----
 6 files changed, 125 insertions(+), 87 deletions(-)

New commits:
diff-tree dd67cf6616c2e819e7e8e4452c1e14e68b4a66bd (from 5a06133eb2e13a4c0354dad7f7da414c85733c4e)
Author: Carl Worth <cworth at raht.cworth.org>
Date:   Tue Apr 4 10:45:38 2006 -0700

    Implement proper cairo-style error-handling for cairo_output_stream_t.
    
    The cairo_output_stream_t object already had an internal status value,
    but it was annoyingly returning status values from various functions.
    It also was missing proper shutdown-on-error as well as nil-create
    semantics.
    
    This fixes those shortcomings and adjusts all callers for the new
    semantics, (leading to simpler and more correct calling
    code---particularly in the case of cairo-base85-stream.c).

diff --git a/src/cairo-base85-stream.c b/src/cairo-base85-stream.c
index 6adfa16..4ea94ea 100644
--- a/src/cairo-base85-stream.c
+++ b/src/cairo-base85-stream.c
@@ -63,19 +63,13 @@ _expand_four_tuple_to_five (unsigned cha
     }
 }
 
-static cairo_status_t
+static void
 _cairo_base85_wrap_perhaps (cairo_base85_stream_t *stream)
 {
-    cairo_status_t status;
-
     if (stream->column >= 72) {
-	status = _cairo_output_stream_write (stream->output, "\n", 1);
-	if (status)
-	    return status;
+	_cairo_output_stream_write (stream->output, "\n", 1);
 	stream->column = 0;
     }
-
-    return CAIRO_STATUS_SUCCESS;
 }
 
 static cairo_status_t
@@ -83,7 +77,6 @@ _cairo_base85_stream_write_data (void			
 				 const unsigned char	*data,
 				 unsigned int		 length)
 {
-    cairo_status_t status;
     cairo_base85_stream_t *stream = closure;
     const unsigned char *ptr = data;
     unsigned char five_tuple[5];
@@ -95,49 +88,38 @@ _cairo_base85_stream_write_data (void			
 	if (stream->pending == 4) {
 	    _expand_four_tuple_to_five (stream->four_tuple, five_tuple, &is_zero);
 	    if (is_zero) {
-		status = _cairo_output_stream_write (stream->output, "z", 1);
+		_cairo_output_stream_write (stream->output, "z", 1);
 		stream->column += 1;
 	    } else {
-		status = _cairo_output_stream_write (stream->output, five_tuple, 5);
+		_cairo_output_stream_write (stream->output, five_tuple, 5);
 		stream->column += 5;
 	    }
-	    if (status)
-		return status;
-	    status = _cairo_base85_wrap_perhaps (stream);
-	    if (status)
-		return status;
+	    _cairo_base85_wrap_perhaps (stream);
 	    stream->pending = 0;
 	}
     }
 
-    return CAIRO_STATUS_SUCCESS;
+    return _cairo_output_stream_get_status (stream->output);
 }
 
 static cairo_status_t
 _cairo_base85_stream_close (void *closure)
 {
-    cairo_status_t status;
     cairo_base85_stream_t *stream = closure;
     unsigned char five_tuple[5];
 
     if (stream->pending) {
 	memset (stream->four_tuple + stream->pending, 0, 4 - stream->pending);
 	_expand_four_tuple_to_five (stream->four_tuple, five_tuple, NULL);
-	status = _cairo_output_stream_write (stream->output, five_tuple, stream->pending + 1);
-	if (status)
-	    return status;
+	_cairo_output_stream_write (stream->output, five_tuple, stream->pending + 1);
 	stream->column += stream->pending + 1;
-	status = _cairo_base85_wrap_perhaps (stream);
-	if (status)
-	    return status;
+	_cairo_base85_wrap_perhaps (stream);
     }
 
     /* Mark end of base85 data */
-    status = _cairo_output_stream_printf (stream->output, "~>\n");
-    if (status)
-	return status;
+    _cairo_output_stream_printf (stream->output, "~>\n");
 
-    return CAIRO_STATUS_SUCCESS;
+    return _cairo_output_stream_get_status (stream->output);
 }
 
 cairo_output_stream_t *
@@ -147,7 +129,7 @@ _cairo_base85_stream_create (cairo_outpu
 
     stream = malloc (sizeof (cairo_base85_stream_t));
     if (stream == NULL)
-	return NULL;
+	return (cairo_output_stream_t *) &cairo_output_stream_nil;
 
     stream->output = output;
     stream->pending = 0;
diff --git a/src/cairo-output-stream.c b/src/cairo-output-stream.c
index 1ed569c..adc00b8 100644
--- a/src/cairo-output-stream.c
+++ b/src/cairo-output-stream.c
@@ -49,6 +49,25 @@ struct _cairo_output_stream {
     void			*closure;
     unsigned long		position;
     cairo_status_t		status;
+    cairo_bool_t		closed;
+};
+
+const cairo_output_stream_t cairo_output_stream_nil = {
+    NULL, /* write_data */
+    NULL, /* close_func */
+    NULL, /* closure */
+    0,    /* position */
+    CAIRO_STATUS_NO_MEMORY,
+    FALSE /* closed */
+};
+
+static const cairo_output_stream_t cairo_output_stream_nil_write_error = {
+    NULL, /* write_data */
+    NULL, /* close_func */
+    NULL, /* closure */
+    0,    /* position */
+    CAIRO_STATUS_WRITE_ERROR,
+    FALSE /* closed */
 };
 
 cairo_output_stream_t *
@@ -60,41 +79,54 @@ _cairo_output_stream_create (cairo_write
 
     stream = malloc (sizeof (cairo_output_stream_t));
     if (stream == NULL)
-	return NULL;
+	return (cairo_output_stream_t *) &cairo_output_stream_nil;
 
     stream->write_data = write_data;
     stream->close_func = close_func;
     stream->closure = closure;
     stream->position = 0;
     stream->status = CAIRO_STATUS_SUCCESS;
+    stream->closed = FALSE;
 
     return stream;
 }
 
-cairo_status_t
-_cairo_output_stream_destroy (cairo_output_stream_t *stream)
+void
+_cairo_output_stream_close (cairo_output_stream_t *stream)
 {
-    cairo_status_t status = CAIRO_STATUS_SUCCESS;
+    cairo_status_t status;
 
-    if (stream->close_func)
+    if (stream->closed)
+	return;
+
+    if (stream->close_func) {
 	status = stream->close_func (stream->closure);
+	if (status)
+	    stream->status = status;
+    }
 
-    free (stream);
+    stream->closed = TRUE;
+}
 
-    return status;
+void
+_cairo_output_stream_destroy (cairo_output_stream_t *stream)
+{
+    _cairo_output_stream_close (stream);
+    free (stream);
 }
 
-cairo_status_t
+void
 _cairo_output_stream_write (cairo_output_stream_t *stream,
 			    const void *data, size_t length)
 {
     if (length == 0)
-	return CAIRO_STATUS_SUCCESS;
+	return;
+
+    if (stream->status)
+	return;
 
     stream->status = stream->write_data (stream->closure, data, length);
     stream->position += length;
-
-    return stream->status;
 }
 
 void
@@ -106,6 +138,9 @@ _cairo_output_stream_write_hex_string (c
     char buffer[2];
     int i, column;
 
+    if (stream->status)
+	return;
+
     for (i = 0, column = 0; i < length; i++, column++) {
 	if (column == 38) {
 	    _cairo_output_stream_write (stream, "\n", 1);
@@ -178,8 +213,7 @@ enum {
  * formatting.  This functionality is only for internal use and we
  * only implement the formats we actually use.
  */
-
-cairo_status_t
+void
 _cairo_output_stream_vprintf (cairo_output_stream_t *stream,
 			      const char *fmt, va_list ap)
 {
@@ -188,6 +222,9 @@ _cairo_output_stream_vprintf (cairo_outp
     const char *f;
     int length_modifier;
 
+    if (stream->status)
+	return;
+
     f = fmt;
     p = buffer;
     while (*f != '\0') {
@@ -250,24 +287,19 @@ _cairo_output_stream_vprintf (cairo_outp
     }
     
     _cairo_output_stream_write (stream, buffer, p - buffer);
-
-    return stream->status;
 }
 
-cairo_status_t
+void
 _cairo_output_stream_printf (cairo_output_stream_t *stream,
 			     const char *fmt, ...)
 {
     va_list ap;
-    cairo_status_t status;    
 
     va_start (ap, fmt);
 
-    status = _cairo_output_stream_vprintf (stream, fmt, ap);
+    _cairo_output_stream_vprintf (stream, fmt, ap);
 
     va_end (ap);
-
-    return status;
 }
 
 long
@@ -312,16 +344,10 @@ cairo_output_stream_t *
 _cairo_output_stream_create_for_file (const char *filename)
 {
     FILE *file;
-    cairo_output_stream_t *stream;
 
     file = fopen (filename, "wb");
     if (file == NULL)
-	return NULL;
+	return (cairo_output_stream_t *) &cairo_output_stream_nil_write_error;
     
-    stream = _cairo_output_stream_create (stdio_write, stdio_close, file);
-
-    if (stream == NULL)
-	fclose (file);
-
-    return stream;
+    return _cairo_output_stream_create (stdio_write, stdio_close, file);
 }
diff --git a/src/cairo-pdf-surface.c b/src/cairo-pdf-surface.c
index 4a783b5..cbccd64 100644
--- a/src/cairo-pdf-surface.c
+++ b/src/cairo-pdf-surface.c
@@ -333,11 +333,13 @@ cairo_pdf_surface_create_for_stream (cai
 				     double			 width_in_points,
 				     double			 height_in_points)
 {
+    cairo_status_t status;
     cairo_output_stream_t *stream;
 
     stream = _cairo_output_stream_create (write, NULL, closure);
-    if (stream == NULL) {
-	_cairo_error (CAIRO_STATUS_NO_MEMORY);
+    status = _cairo_output_stream_get_status (stream);
+    if (status) {
+	_cairo_error (status);
 	return (cairo_surface_t*) &_cairo_surface_nil;
     }
 
@@ -368,11 +370,13 @@ cairo_pdf_surface_create (const char		*f
 			  double		 width_in_points,
 			  double		 height_in_points)
 {
+    cairo_status_t status;
     cairo_output_stream_t *stream;
 
     stream = _cairo_output_stream_create_for_file (filename);
-    if (stream == NULL) {
-	_cairo_error (CAIRO_STATUS_NO_MEMORY);
+    status = _cairo_output_stream_get_status (stream);
+    if (status) {
+	_cairo_error (status);
 	return (cairo_surface_t*) &_cairo_surface_nil;
     }
 
diff --git a/src/cairo-ps-surface.c b/src/cairo-ps-surface.c
index 4242a1e..7689205 100644
--- a/src/cairo-ps-surface.c
+++ b/src/cairo-ps-surface.c
@@ -178,11 +178,13 @@ cairo_ps_surface_create (const char		*fi
 			 double			 width_in_points,
 			 double			 height_in_points)
 {
+    cairo_status_t status;
     cairo_output_stream_t *stream;
 
     stream = _cairo_output_stream_create_for_file (filename);
-    if (stream == NULL) {
-	_cairo_error (CAIRO_STATUS_NO_MEMORY);
+    status = _cairo_output_stream_get_status (stream);
+    if (status) {
+	_cairo_error (status);
 	return (cairo_surface_t*) &_cairo_surface_nil;
     }
 
@@ -216,11 +218,13 @@ cairo_ps_surface_create_for_stream (cair
 				    double		width_in_points,
 				    double		height_in_points)
 {
+    cairo_status_t status;
     cairo_output_stream_t *stream;
 
     stream = _cairo_output_stream_create (write_func, NULL, closure);
-    if (stream == NULL) {
-	_cairo_error (CAIRO_STATUS_NO_MEMORY);
+    status = _cairo_output_stream_get_status (stream);
+    if (status) {
+	_cairo_error (status);
 	return (cairo_surface_t*) &_cairo_surface_nil;
     }
 
@@ -758,18 +762,13 @@ emit_image (cairo_ps_surface_t    *surfa
 
     /* Compressed image data (Base85 encoded) */
     base85_stream = _cairo_base85_stream_create (surface->stream);
-    if (base85_stream == NULL) {
-	status = CAIRO_STATUS_NO_MEMORY;
-	goto bail3;
-    }
 
-    status = _cairo_output_stream_write (base85_stream, compressed, compressed_size);
-    if (status) {
-	_cairo_output_stream_destroy (base85_stream);
-	goto bail3;
-    }
+    _cairo_output_stream_write (base85_stream, compressed, compressed_size);
+    _cairo_output_stream_close (base85_stream);
+
+    status = _cairo_output_stream_get_status (base85_stream);
 
-    status = _cairo_output_stream_destroy (base85_stream);
+    _cairo_output_stream_destroy (base85_stream);
 
  bail3:
     free (compressed);
diff --git a/src/cairo-svg-surface.c b/src/cairo-svg-surface.c
index 7c38464..253055a 100644
--- a/src/cairo-svg-surface.c
+++ b/src/cairo-svg-surface.c
@@ -143,11 +143,15 @@ cairo_svg_surface_create_for_stream (cai
 				     double			width,
 				     double			height)
 {
+    cairo_status_t status;
     cairo_output_stream_t *stream;
 
     stream = _cairo_output_stream_create (write, NULL, closure);
-    if (stream == NULL)
-	return NULL;
+    status = _cairo_output_stream_get_status (stream);
+    if (status) {
+	_cairo_error (status);
+	return (cairo_surface_t *) &cairo_surface_nil;
+    }
 
     return _cairo_svg_surface_create_for_stream_internal (stream, width, height);
 }
@@ -157,11 +161,15 @@ cairo_svg_surface_create (const char	*fi
 			  double	width,
 			  double	height)
 {
+    cairo_status_t status;
     cairo_output_stream_t *stream;
 
     stream = _cairo_output_stream_create_for_file (filename);
-    if (stream == NULL)
-	return NULL;
+    status = _cairo_output_stream_get_status (stream);
+    if (status) {
+	_cairo_error (status);
+	return (cairo_surface_t *) &cairo_surface_nil;
+    }
 
     return _cairo_svg_surface_create_for_stream_internal (stream, width, height);
 }
diff --git a/src/cairoint.h b/src/cairoint.h
index 4203599..37998b7 100644
--- a/src/cairoint.h
+++ b/src/cairoint.h
@@ -2124,6 +2124,8 @@ _cairo_utf8_to_utf16 (const unsigned cha
 
 typedef struct _cairo_output_stream cairo_output_stream_t;
 
+extern const cairo_private cairo_output_stream_t cairo_output_stream_nil;
+
 /* We already have the following declared in cairo.h:
 
 typedef cairo_status_t (*cairo_write_func_t) (void		  *closure,
@@ -2132,18 +2134,27 @@ typedef cairo_status_t (*cairo_write_fun
 */
 typedef cairo_status_t (*cairo_close_func_t) (void *closure);
 
+
+/* This function never returns NULL. If an error occurs (NO_MEMORY)
+ * while trying to create the output stream this function returns a
+ * valid pointer to a nil output stream.
+ *
+ * Note that even with a nil surface, the close_func callback will be
+ * called by a call to _cairo_output_stream_close or
+ * _cairo_output_stream_destroy.
+ */
 cairo_private cairo_output_stream_t *
 _cairo_output_stream_create (cairo_write_func_t		write_func,
 			     cairo_close_func_t		close_func,
 			     void			*closure);
 
-/* Most cairo destroy functions don't return a status, but we do here
- * to allow the return status from the close_func callback to be
- * captured. */
-cairo_private cairo_status_t
+cairo_private void
+_cairo_output_stream_close (cairo_output_stream_t *stream);
+
+cairo_private void
 _cairo_output_stream_destroy (cairo_output_stream_t *stream);
 
-cairo_private cairo_status_t
+cairo_private void
 _cairo_output_stream_write (cairo_output_stream_t *stream,
 			    const void *data, size_t length);
 
@@ -2152,14 +2163,14 @@ _cairo_output_stream_write_hex_string (c
 				       const char *data,
 				       size_t length);
 
-unsigned char *
+cairo_private unsigned char *
 _cairo_lzw_compress (unsigned char *data, unsigned long *data_size_in_out);
 
-cairo_private cairo_status_t
+cairo_private void
 _cairo_output_stream_vprintf (cairo_output_stream_t *stream,
 			      const char *fmt, va_list ap);
 
-cairo_private cairo_status_t
+cairo_private void
 _cairo_output_stream_printf (cairo_output_stream_t *stream,
 			     const char *fmt, ...) CAIRO_PRINTF_FORMAT(2, 3);
 
@@ -2169,6 +2180,14 @@ _cairo_output_stream_get_position (cairo
 cairo_private cairo_status_t
 _cairo_output_stream_get_status (cairo_output_stream_t *stream);
 
+/* This function never returns NULL. If an error occurs (NO_MEMORY or
+ * WRITE_ERROR) while trying to create the output stream this function
+ * returns a valid pointer to a nil output stream.
+ *
+ * NOTE: Even if a nil surface is returned, the caller should still
+ * call _cairo_output_stream_destroy (or _cairo_output_stream_close at
+ * least) in order to ensure that everything is properly cleaned up.
+ */
 cairo_private cairo_output_stream_t *
 _cairo_output_stream_create_for_file (const char *filename);
 


More information about the cairo-commit mailing list