[Spice-devel] [PATCH v6 21/26] spice-gtk: Enable adding alternative video decoders

[Francois Gouget]

On Fri, 16 Oct 2015, Victor Toso wrote:
[...]
> > -    cinfo->src->bytes_in_buffer = stream_get_current_frame(st, &data);
> > +    uint8_t *data;
> > +    cinfo->src->bytes_in_buffer = spice_msg_in_frame_data(decoder->frame_msg, &data);
> >      cinfo->src->next_input_byte = data;
> 
> data could be not initialized here in case of error and
> spice_msg_in_frame_data does not set it to NULL.
> Either one should do it.
> 
> ** stream_get_current_frame was doing it before

Actually it didn't quite.

It set data to NULL in case msg_data was NULL. But I consider not 
passing a NULL pointer to spice_msg_in_frame_data() to be the 
responsibility of the caller since it would not make sense. In any case 
the g_return_val_if_fail(in != NULL, FALSE) check in 
display_stream_render() makes this case impossible, probably more 
clearly now that we're not using a stream member variable that almost 
anyone can access.

But in the other error case stream_get_current_frame() was not 
initializing data. So I propose the attached two-patch set.

p21a.diff makes sure stream_get_current_frame() always initializes data.
p21b.diff is the updated patch preserving this behavior in 
          spice_msg_in_frame_data().

As far as I can tell the remainder of the patches should still apply 
fine.


-- 
Francois Gouget <fgouget at codeweavers.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: p21a.diff
Type: text/x-diff
Size: 1279 bytes
Desc: p21a.diff 
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20151016/d2e54112/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: p21b.diff
Type: text/x-diff
Size: 16859 bytes
Desc: p21b.diff
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20151016/d2e54112/attachment-0001.diff>