[cairo] SUMMARY of Implementing cairo_read_func_t/cairo_write_func_t properly

[Bryce Harrington]

On Thu, Mar 03, 2016 at 03:52:12PM +0100, Bernhard R. Fischer wrote:
> On 2016-03-01 06:58, Bernhard R. Fischer wrote:
> > The conclusion of this discussion was to slightly adapt the meaning of
> > the return value of the read_func_t as follows:
> > 
> > 
> > The read function shall return the negative number of bytes which have
> > NOT been read, i.e. - (length - bytes_read).
> > This is that a full read will return 0 which is CAIRO_STATUS_SUCCESS.
> > Truncated reads return something between -1 and -length. On error,
> > CAIRO_STATUS_READ_ERROR is returned (which is a positive value).
> > 
> > 
> > 
> > If there is the consent of the core devs we should adapt the docs.
> > 
> > I'll have a look at cairo_image_surface_create_from_png_stream() if
> > there is a change necessary.
> > 
> > Bernhard
> > 
> 
> 
> 
> As promised, I just had a look at implementation of the PNG code of
> Cairo (cairo-png.c).
> 
> If a read_func() returns a negative value as suggested, the whole
> program will be aborted because of a call to assert() in
> _cairo_surface_create_in_error() (cairo-surface.c) which in turn is
> called by the PNG error handler png_simple_error_callback() in cairo-png.c.
> 
> 
> I suggest the following tiny patch (to cairo-1.14.6) to fix this:
> 
> 
> *** cairo-png_orig.c    2016-03-03 15:41:35.565400515 +0100
> --- cairo-png.c 2016-03-03 15:42:05.301547969 +0100
> ***************
> *** 525,530 ****
> --- 525,531 ----
>       png_closure = png_get_io_ptr (png);
>       status = png_closure->read_func (png_closure->closure, data, size);
>       if (unlikely (status)) {
> +        if ((int) status < 0) status = CAIRO_STATUS_READ_ERROR;
>         cairo_status_t *error = png_get_error_ptr (png);
>         if (*error == CAIRO_STATUS_SUCCESS)
>             *error = status;

This would probably need to look more like:

    status = png_closure->read_func (png_closure->closure, data, size);
    if (unlikely (status)) {
        cairo_status_t *error;
        if ((int) status < 0)
            status = CAIRO_STATUS_READ_ERROR;
        error = png_get_error_ptr (png);

Was there also a discussion of instead of overloading the return value
of the read_func_t to add a &size parameter to its arguments for
returning the number of bytes read?

Bryce