[Spice-devel] [PATCH v3 5/9] Add new spice-gl stubs API

[Marc-André Lureau]

Hi

On Thu, Feb 4, 2016 at 4:29 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
>> +{
>> +    spice_return_if_fail(qxl != NULL);
>> +    spice_return_if_fail(qxl->st->gl_draw_async == NULL);
>
> Here you need to close fd on error or you will get a file
> descriptor leak.

>> +
>> +SPICE_GNUC_VISIBLE
>> +void spice_gl_draw_async(QXLInstance *qxl,
>> +                         uint32_t x, uint32_t y,
>> +                         uint32_t w, uint32_t h,
>> +                         uint64_t cookie)
>> +{
>> +    spice_return_if_fail(qxl != NULL);
>> +    spice_return_if_fail(qxl->st->scanout.drm_dma_buf_fd != -1);
>> +    spice_return_if_fail(qxl->st->gl_draw_async == NULL);
>
> Here you should signal the cookie (or return an error to be handled by the
> caller or rendering will stuck

Those two are pre-conditions, and abort() by default. (even if
DISABLE_ABORT is set, they should never happen at runtime) Hence it's
okay to not handle this error case. If you start handling it, then it
becomes implicit that you deal with those cases and you'll have to
keep it, but it should never have to in the first place. This is how
most of glib libraries/applications are written. Since qemu relies on
glib, they have the same contract with glib libraries, although
internally they most often use abort() pre-conditions (as spice server
by default).

-- 
Marc-André Lureau