Re: [PATCH] Generate better error message when loading invalid PNGs

Am 25.03.2016 um 22:11 schrieb Adrian Johnson:
> On 26/03/16 03:53, Uli Schlachter wrote:
>> Based on an idea from Cyril Roelandt, this patch makes cairo generate better
>> error messages when cairo_image_surface_create_from_png{,_stream} is called on
>> e.g. JPEG files. To do so, we don't let libpng check if the file starts with a
>> PNG signature, but do so by hand. On mismatch, a surface with status
>> CAIRO_STATUS_READ_ERROR is returned where previously the status was
>> CAIRO_STATUS_NO_MEMORY (the status used for all errors in libpng).
> 
> What about if it is a PNG file but libpng is unable to decode it? It is
> still going to return the unhelpful CAIRO_STATUS_NO_MEMORY. It is also
> useful restrict CAIRO_STATUS_READ_ERROR to errors reading the from the
> stream.
> 
> I think we need an error status for backends that can fail. The attached
> patch adds CAIRO_STATUS_PNG_ERROR. I can also write a patch for win32 as
> it is returning CAIRO_STATUS_NO_MEMORY for GDI errors.

Fine with me. Note that we could detect OOM reliably by making libpng use our
own malloc/free wrappers via png_create_{read,write}_struct_2(). However,
perhaps this is a bit excessive.

> @@ -744,6 +746,7 @@ read_png (struct png_read_closure_t *png_closure)
>   *	%CAIRO_STATUS_NO_MEMORY
>   *	%CAIRO_STATUS_FILE_NOT_FOUND
>   *	%CAIRO_STATUS_READ_ERROR
> + *      %CAIRO_STATUS_PNG_ERROR
>   *
>   * Alternatively, you can allow errors to propagate through the drawing
>   * operations and check the status on the context upon completion
> @@ -799,6 +802,7 @@ cairo_image_surface_create_from_png (const char *filename)
>   *
>   *	%CAIRO_STATUS_NO_MEMORY
>   *	%CAIRO_STATUS_READ_ERROR
> + *      %CAIRO_STATUS_PNG_ERROR
>   *
>   * Alternatively, you can allow errors to propagate through the drawing
>   * operations and check the status on the context upon completion

In both of the above hunks: The code around uses a tab while your patch adds spaces.

With the above fixed:

Reviewed-by: Uli Schlachter <[email protected]>

(Should there also be matching unit test, similar to what my patch did?)

Cheers,
Uli
-- 
"Do you know that books smell like nutmeg or some spice from a foreign land?"
                                                  -- Faber in Fahrenheit 451
-- 
cairo mailing list
[email protected]
https://lists.cairographics.org/mailman/listinfo/cairo