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