Re: [PATCH wayland v3 04/10] connection: Clear fds we shouldn't close to -1

[Daniel Stone]

Hi Derek,

On 6 December 2017 at 17:22, Derek Foreman  wrote:
> This initializes all the fd arguments in closures to -1 and clears
> them back to -1 when they've been dispatched or serialized.
>
> This means that any valid fd in a closure is currently libwayland's
> responsibility to close in the case of an error.

As with the previous, I'm totally happy with this and intend to land,
but a good follow-on might be to have wl_closure_{invoke,dispatch}
take full ownership of the closure, i.e. unconditionally destroy it
before return. All its users do this anyway.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland v3 04/10] connection: Clear fds we shouldn't close to -1

[Derek Foreman]

This initializes all the fd arguments in closures to -1 and clears
them back to -1 when they've been dispatched or serialized.

This means that any valid fd in a closure is currently libwayland's
responsibility to close in the case of an error.

Signed-off-by: Derek Foreman 
---
 src/connection.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/src/connection.c b/src/connection.c
index 8d8eb60..29f565b 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -524,6 +524,17 @@ wl_argument_from_va_list(const char *signature, union 
wl_argument *args,
}
 }
 
+static void
+wl_closure_clear_fds(struct wl_closure *closure)
+{
+   int i;
+
+   for (i = 0; closure->message->signature[i]; i++) {
+   if (closure->message->signature[i] == 'h')
+   closure->args[i].h = -1;
+   }
+}
+
 static struct wl_closure *
 wl_closure_init(const struct wl_message *message, uint32_t size,
 int *num_arrays, union wl_argument *args)
@@ -557,6 +568,14 @@ wl_closure_init(const struct wl_message *message, uint32_t 
size,
closure->message = message;
closure->count = count;
 
+   /* Set these all to -1 so we can close any that have been
+* set to a real value during wl_closure_destroy().
+* We may have copied a bunch of fds into the closure with
+* memcpy previously, but those are undup()d client fds
+* that we would have replaced anyway.
+*/
+   wl_closure_clear_fds(closure);
+
return closure;
 }
 
@@ -948,6 +967,8 @@ wl_closure_invoke(struct wl_closure *closure, uint32_t 
flags,
 opcode, target->interface->name);
}
ffi_call(, implementation[opcode], NULL, ffi_args);
+
+   wl_closure_clear_fds(closure);
 }
 
 void
@@ -956,6 +977,8 @@ wl_closure_dispatch(struct wl_closure *closure, 
wl_dispatcher_func_t dispatcher,
 {
dispatcher(target->implementation, target, opcode, closure->message,
   closure->args);
+
+   wl_closure_clear_fds(closure);
 }
 
 static int
@@ -980,6 +1003,7 @@ copy_fds_to_connection(struct wl_closure *closure,
   "can't send file descriptor");
return -1;
}
+   closure->args[i].h = -1;
}
 
return 0;
-- 
2.15.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel