[PATCH wayland] Portability: Simple stability improvements and cleanups

[Pekka Paalanen]

On Thu, 11 Sep 2014 00:02:48 +0200
Karsten Otto <karsten.otto at posteo.de> wrote:

> A selection of simple patches to improve general stability of wayland.
> They are harmless enough to be included anytime, maybe even for 1.6, but no rush.
> Tested on ubuntu 14.04 LTS with wayland/weston (>1.5.92)
> 
> The patches were originally posted by Philip Withnall in his series of FreeBSD portability patches:
> 
> [PATCH 01/12] connection: Fix sendmsg() on FreeBSD
> http://lists.freedesktop.org/archives/wayland-devel/2013-February/007441.html
> [PATCH 06/12] event-loop.c: Use correct OS abstraction function for dupfd()
> http://lists.freedesktop.org/archives/wayland-devel/2013-February/007446.html
> [PATCH 08/12] wayland-server: Abort if a read from a client gives 0 length
> http://lists.freedesktop.org/archives/wayland-devel/2013-February/007448.html
> [PATCH 11/12] queue-test: Add another assertion
> http://lists.freedesktop.org/archives/wayland-devel/2013-February/007451.html
> 
> (I fixed the last one to use assert instead of client_assert).
> 
> ---
> src/connection.c     | 13 +++++++++++--
> src/event-loop.c     |  2 +-
> src/wayland-server.c |  2 +-
> tests/queue-test.c   |  2 +-
> 4 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/src/connection.c b/src/connection.c
> index f292853..c4eb354 100644
> --- a/src/connection.c
> +++ b/src/connection.c
> @@ -287,10 +287,19 @@ wl_connection_flush(struct wl_connection *connection)
> 		msg.msg_namelen = 0;
> 		msg.msg_iov = iov;
> 		msg.msg_iovlen = count;
> -		msg.msg_control = cmsg;
> -		msg.msg_controllen = clen;
> +		msg.msg_control = NULL;
> +		msg.msg_controllen = 0;
> 		msg.msg_flags = 0;
> 
> +		/* FreeBSD requires msg_control to be set to NULL iff
> +		 * msg_controllen is 0 (see
> +		 * http://www.freebsd.org/cgi/query-pr.cgi?pr=docs/99356#reply2)
> +		 * Can't hurt to do that on all platforms. */
> +		if (clen > 0) {
> +			msg.msg_controllen = clen;
> +			msg.msg_control = cmsg;
> +		}
> +
> 		do {
> 			len = sendmsg(connection->fd, &msg,
> 				      MSG_NOSIGNAL | MSG_DONTWAIT);
> diff --git a/src/event-loop.c b/src/event-loop.c
> index a149db9..1f571ba 100644
> --- a/src/event-loop.c
> +++ b/src/event-loop.c
> @@ -134,7 +134,7 @@ wl_event_loop_add_fd(struct wl_event_loop *loop,
> 		return NULL;
> 
> 	source->base.interface = &fd_source_interface;
> -	source->base.fd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
> +	source->base.fd = wl_os_dupfd_cloexec(fd, 0);
> 	source->func = func;
> 	source->fd = fd;
> 
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 674aeca..83e6f83 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -260,7 +260,7 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
> 	len = 0;
> 	if (mask & WL_EVENT_READABLE) {
> 		len = wl_connection_read(connection);
> -		if (len < 0 && errno != EAGAIN) {
> +		if (len <= 0 && errno != EAGAIN) {
> 			wl_client_destroy(client);
> 			return 1;
> 		}
> diff --git a/tests/queue-test.c b/tests/queue-test.c
> index 96f2100..6e2e932 100644
> --- a/tests/queue-test.c
> +++ b/tests/queue-test.c
> @@ -66,7 +66,7 @@ client_test_proxy_destroy(void)
> 	assert(registry != NULL);
> 	wl_registry_add_listener(registry, &registry_listener,
> 				 &counter);
> -	wl_display_roundtrip(display);
> +	assert(wl_display_roundtrip(display) != -1);
> 
> 	assert(counter == 1);
> 

Hi,

I think every file changed here should be a separate patch, since the
changes are all over the place: connection.c is common to server and
client, event-loop is an optional part of server, tests, etc.

Would you mind keeping the original patch split and the detailed commit
messages, and just rebase the relevant patches to master? That would
also keep the author info. You can record your own contribution with
something like e.g.
http://cgit.freedesktop.org/wayland/weston/commit/?id=d8deff617a18aa71a0501a52569b2328706cf35a

I'll queue them for 1.7 then.


Thanks,
pq