[PATCH v2 wayland 04/11] connection: Make wl_closure_destroy() close fds of undispatched closures

[Daniel Stone]

Hi Derek,

On 13 April 2017 at 17:51, Derek Foreman <derekf at osg.samsung.com> wrote:
>  void
> -wl_closure_destroy(struct wl_closure *closure)
> +wl_closure_destroy(struct wl_closure *closure, bool dispatched)
>  {
> +       /* wl_closure_destroy has free() semantics */
> +       if (!closure)
> +               return;
> +       /* If message is NULL then this closure failed during
> +        * creation, and any fd cleanup has been done by the
> +        * caller
> +        */
> +       if (!dispatched && closure->message)
> +               wl_closure_close_fds(closure);
>         free(closure);
>  }

Maybe there's something I'm missing here, but is there any reason to
not set closure->message unconditionally? If you moved the
closure->message assignment up inside marshal/demarshal, and ran
through initialising all the handle-type arg values to -1 (let's call
this hypothetical function wl_closure_init - take an 'extra' size
param to preserve the single-alloc demarshal semantics as well), then
you could just use this everywhere rather than manual fd counting.

Also, please use an enum rather than true/false: it was only after I
got to like four 'this seems completely backwards?' comments that I
realised true meant 'don't do the new behaviour', and false meant 'do
extra stuff'. I blame the flu medication, but still ...

Cheers,
Daniel