[cairo] [PATCH] Improve error message when using cairo_image_surface_create_from_png with an invalid file.

[Cyril Roelandt]

Hello!

Consider the following example program:

#include <stdio.h>
#include <cairo.h>

int
main(int argc, char *argv[])
{
	cairo_surface_t *img = NULL;
	const char *path = "/tmp/foo.jpg";
	int i;

	img = cairo_image_surface_create_from_png(path);
	if (cairo_surface_status(img) != CAIRO_STATUS_SUCCESS) {
		fprintf(stderr, "Status: %s\n",
			cairo_status_to_string(cairo_surface_status(img)));
	}
	return 0;
}

With /tmp/foo.jpg being a valid, readable JPEG file. The output of the
program is:

"Status: out of memory"

which is not really a description of the error that occurred (the file
should have been a PNG image). This is an obvious issue when writing a
program using cairo, but it is far less obvious when running an
application that uses cairo behind the scenes. For instance, i3lock, a
screen locker, allows users to display an image of their choosing on the
locked screen, but only accepts PNGs. When mistakenly providing a JPEG
file, end users are usually a bit confused by the error message[1].

The error status is set to CAIRO_STATUS_NO_MEMORY as a "default" error
in png_simple_error_callback(), since png_get_error_ptr() returns
CAIRO_STATUS_SUCCESS. However, I do not think we are really unable to
find out what happened: a dirty but working heuristic would be to look
at the value of the error message passed to the error callback. In our
case, it clearly states "Not a PNG file".

The following patch, or something along those lines, might help users
get a better error message:

diff --git a/src/cairo-png.c b/src/cairo-png.c
index 068617d..e87c465 100644
--- a/src/cairo-png.c
+++ b/src/cairo-png.c
@@ -134,9 +134,15 @@ png_simple_error_callback (png_structp png,
 {
     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);
+    if (*error == CAIRO_STATUS_SUCCESS) {
+        /* Try do determine the error using the error message.
+           If not possible, default to the most likely error. */
+        if (!strcmp(error_msg, "Not a PNG file"))
+            *error = CAIRO_STATUS_READ_ERROR;
+        else
+           *error = CAIRO_STATUS_NO_MEMORY;
+        *error = _cairo_error(*error);
+    }

 #ifdef PNG_SETJMP_SUPPORTED
     longjmp (png_jmpbuf (png), 1);


Maybe we could even have a CAIRO_STATUS_NOT_A_PNG error status. What do
you think?

Cyril Roelandt.

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=776682