[cairo] SUMMARY of Implementing cairo_read_func_t/cairo_write_func_t properly

[Bernhard R. Fischer]

On 2016-03-09 18:20, Bill Spitzak wrote:
> On Wed, Mar 9, 2016 at 12:30 AM, Bernhard R. Fischer <bf at abenteuerland.at>
> wrote:
> 
>> On 2016-03-04 19:16, Bryce Harrington wrote:
>>> 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);
>>
>>
>> Why? It makes no difference.
>>
> 
> The variables have to be declared first before any statements (required by
> older C compilers).
> 


I just inserted the 1-line "if ( )..." statement. Everything else is as
it is in the current (1.14.6) cairo-png.c.


Bernhard