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).

src/cairo-base85-stream.c

@@ -63,19 +63,13 @@ _expand_four_tuple_to_five (unsigned char four_tuple[4],
     }
 }
 
-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);
+	_cairo_output_stream_write (stream->output, "\n", 1);
-	if (status)
-	    return status;
 	stream->column = 0;
     }
-
-    return CAIRO_STATUS_SUCCESS;
 }
 
 static cairo_status_t
@@ -83,7 +77,6 @@ _cairo_base85_stream_write_data (void			*closure,
 				 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			*closure,
 	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)
+	    _cairo_base85_wrap_perhaps (stream);
-		return status;
-	    status = _cairo_base85_wrap_perhaps (stream);
-	    if (status)
-		return status;
 	    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);
+	_cairo_output_stream_write (stream->output, five_tuple, stream->pending + 1);
-	if (status)
-	    return status;
 	stream->column += stream->pending + 1;
-	status = _cairo_base85_wrap_perhaps (stream);
+	_cairo_base85_wrap_perhaps (stream);
-	if (status)
-	    return status;
     }
 
     /* Mark end of base85 data */
-    status = _cairo_output_stream_printf (stream->output, "~>\n");
+    _cairo_output_stream_printf (stream->output, "~>\n");
-    if (status)
-	return status;
 
-    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_output_stream_t *output)
 
     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;

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_func_t		write_data,
 
     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
+void
-_cairo_output_stream_destroy (cairo_output_stream_t *stream)
+_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;
 }
 
-cairo_status_t
+void
+_cairo_output_stream_destroy (cairo_output_stream_t *stream)
+{
+    _cairo_output_stream_close (stream);
+    free (stream);
+}
+
+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 (cairo_output_stream_t *stream,
     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.
  */
-
+void
-cairo_status_t
 _cairo_output_stream_vprintf (cairo_output_stream_t *stream,
 			      const char *fmt, va_list ap)
 {
@@ -188,6 +222,9 @@ _cairo_output_stream_vprintf (cairo_output_stream_t *stream,
     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_output_stream_t *stream,
     }
     
     _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);
+    return _cairo_output_stream_create (stdio_write, stdio_close, file);
-
-    if (stream == NULL)
-	fclose (file);
-
-    return stream;
 }

src/cairo-pdf-surface.c

@@ -333,11 +333,13 @@ cairo_pdf_surface_create_for_stream (cairo_write_func_t		 write,
 				     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) {
+    status = _cairo_output_stream_get_status (stream);
-	_cairo_error (CAIRO_STATUS_NO_MEMORY);
+    if (status) {
+	_cairo_error (status);
 	return (cairo_surface_t*) &_cairo_surface_nil;
     }
 
@@ -368,11 +370,13 @@ cairo_pdf_surface_create (const char		*filename,
 			  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) {
+    status = _cairo_output_stream_get_status (stream);
-	_cairo_error (CAIRO_STATUS_NO_MEMORY);
+    if (status) {
+	_cairo_error (status);
 	return (cairo_surface_t*) &_cairo_surface_nil;
     }
 

src/cairo-ps-surface.c

@@ -178,11 +178,13 @@ cairo_ps_surface_create (const char		*filename,
 			 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) {
+    status = _cairo_output_stream_get_status (stream);
-	_cairo_error (CAIRO_STATUS_NO_MEMORY);
+    if (status) {
+	_cairo_error (status);
 	return (cairo_surface_t*) &_cairo_surface_nil;
     }
 
@@ -216,11 +218,13 @@ cairo_ps_surface_create_for_stream (cairo_write_func_t	write_func,
 				    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) {
+    status = _cairo_output_stream_get_status (stream);
-	_cairo_error (CAIRO_STATUS_NO_MEMORY);
+    if (status) {
+	_cairo_error (status);
 	return (cairo_surface_t*) &_cairo_surface_nil;
     }
 
@@ -758,18 +762,13 @@ emit_image (cairo_ps_surface_t    *surface,
 
     /* 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);
+    _cairo_output_stream_write (base85_stream, compressed, compressed_size);
-    if (status) {
+    _cairo_output_stream_close (base85_stream);
-	_cairo_output_stream_destroy (base85_stream);
-	goto bail3;
-    }
 
-    status = _cairo_output_stream_destroy (base85_stream);
+    status = _cairo_output_stream_get_status (base85_stream);
+
+    _cairo_output_stream_destroy (base85_stream);
 
  bail3:
     free (compressed);

src/cairo-svg-surface.c

@@ -143,11 +143,15 @@ cairo_svg_surface_create_for_stream (cairo_write_func_t		write,
 				     double			width,
 				     double			height)
 {
+    cairo_status_t status;
     cairo_output_stream_t *stream;
 
     stream = _cairo_output_stream_create (write, NULL, closure);
-    if (stream == NULL)
+    status = _cairo_output_stream_get_status (stream);
-	return NULL;
+    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	*filename,
 			  double	width,
 			  double	height)
 {
+    cairo_status_t status;
     cairo_output_stream_t *stream;
 
     stream = _cairo_output_stream_create_for_file (filename);
-    if (stream == NULL)
+    status = _cairo_output_stream_get_status (stream);
-	return NULL;
+    if (status) {
+	_cairo_error (status);
+	return (cairo_surface_t *) &cairo_surface_nil;
+    }
 
     return _cairo_svg_surface_create_for_stream_internal (stream, width, height);
 }

src/cairoint.h

@@ -2124,6 +2124,8 @@ _cairo_utf8_to_utf16 (const unsigned char *str,
 
 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_func_t) (void		  *closure,
 */
 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
+cairo_private void
- * to allow the return status from the close_func callback to be
+_cairo_output_stream_close (cairo_output_stream_t *stream);
- * captured. */
+
-cairo_private cairo_status_t
+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 (cairo_output_stream_t *stream,
 				       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_output_stream_t *status);
 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);