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

Cyril Roelandt <[email protected]>
Newsgroups gmane.comp.lib.cairo
Message-ID <[email protected]>
On 03/23/2016 02:37 PM, Uli Schlachter wrote:

> 
> I don't like "grepping" through error messages like this (what if libpng changes 
> the exact wording?) and wondered if there is no better API for this. For the 
> first time in my life I looked at the libpng API (and the API docs) and I hope 
> that I will never do that again...

I don't like grepping either, that's why I called this "dirty" :)

> So second try was to search for "Not a PNG file" in debian codesearch where I 
> found the following:
> 
> http://sources.debian.net/src/vtk/5.10.1+dfsg-2/IO/vtkPNGReader.cxx/?hl=58#L58
> 
> This reads the first 8 bytes of the file (handles short reads correctly!) and 
> uses png_sig_cmp() to check that these bytes are correct (and then it uses 
> png_set_sig_bytes() in line 101 to compensate for these missing bytes).
> 

Couldn't we read the first 8 bytes of the file in
cairo_image_surface_create_from_png() before calling read_png() ? If the
file is not a PNG, then we could return an error code
(CAIRO_STATUS_INVALID_FILE or whatever); if it is a PNG, then we'd pass
it to libpng, which would not fail.

> Having seen this code, I like libpng's API even less, but apparently this would 
> be the "official" way to handle this.
> 
> Perhabs checking the error message isn't so bad after all... I'm unsure.
> 
>  > Maybe we could even have a CAIRO_STATUS_NOT_A_PNG error status. What do
>  > you think?
> 
> The PNG-API in cairo is part of the toy interfaces, which exist to simplify 
> writing examples and tests. They aren't meant to offer all possibilities (use 
> something like gdk-pixbuf for that).
> 
> Thus, I don't think we should have special error codes for the toy API (but I 
> don't mind if someone thinks otherwise and adds them anyway).
> 

I'll trust you on that since I do not really know cairo :)


Cheers,
Cyril.
-- 
cairo mailing list
[email protected]
https://lists.cairographics.org/mailman/listinfo/cairo
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.