>
> 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).
>
Yes, all this is quite messy.
I don't consider this a stop over anyway.
Actually spice log functions and glib ones are not 100% compatible by
definition and settings.
That's also why it's hard to came to some final decisions.
The spice_return is more of a g_assert but without the possibility of
removing these checks.
The g_return_if is more of an if with logs.
That's actually why spice_assert is more used and understood (as more
similar to g_assert... except the disabling part!)
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel