[cairo] [PATCH] Improve error message when using cairo_image_surface_create_from_png with an invalid file.
Cyril Roelandt
tipecaml at gmail.com
Tue Mar 22 22:19:53 UTC 2016
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
More information about the cairo
mailing list