Re: [PATCH v3 5/9] Add new spice-gl stubs API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

On Thu, Feb 4, 2016 at 4:29 PM, Frediano Ziglio <fziglio@xxxxxxxxxx> 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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]