[PATCH wayland 8/8] client: Use read preparation API in wl_display_dispatch_queue()

[Bryce Harrington]

On Fri, Oct 02, 2015 at 05:32:59PM +0800, Jonas Ådahl wrote:
> Instead of doing things that do the equivalent of using
> wl_display_prepare_read() and friends, just use the public API. The
> only semantical difference is that we will now unlock and lock the mutex
> more times compared to before.
> 
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
> 
> There is no real need to merge this patch, it only makes it easier to
> understand that the _dispatch(_queue)() functions are compatible with the
> prepare_read() family.

I'm all for making code more concise and clear.  Is there any concern
though that the added locking activity impacts performance negatively?

Technically I see nothing wrong with the implementation.

> Jonas
> 
> 
>  src/wayland-client.c | 35 ++++++++---------------------------
>  1 file changed, 8 insertions(+), 27 deletions(-)
> 
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index ca2495f..8f53f6f 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -1507,15 +1507,9 @@ wl_display_dispatch_queue(struct wl_display *display,
>  	struct pollfd pfd[2];
>  	int ret;
>  
> -	pthread_mutex_lock(&display->mutex);
> -
> -	ret = dispatch_queue(display, queue);
> -	if (ret == -1)
> -		goto err_unlock;
> -	if (ret > 0) {
> -		pthread_mutex_unlock(&display->mutex);
> +	ret = wl_display_dispatch_queue_pending(display, queue);
> +	if (ret != 0)
>  		return ret;
> -	}
>  
>  	/* We ignore EPIPE here, so that we try to read events before
>  	 * returning an error.  When the compositor sends an error it
> @@ -1524,12 +1518,11 @@ wl_display_dispatch_queue(struct wl_display *display,
>  	ret = wl_connection_flush(display->connection);
>  	if (ret < 0 && errno != EAGAIN && errno != EPIPE) {
>  		display_fatal_error(display, errno);
> -		goto err_unlock;
> +		return -1;
>  	}
>  
> -	display->reader_count++;
> -
> -	pthread_mutex_unlock(&display->mutex);
> +	if (wl_display_prepare_read_queue(display, queue) != 0)
> +		return -1;
>  
>  	pfd[0].fd = display->fd;
>  	pfd[0].events = POLLIN;
> @@ -1542,22 +1535,10 @@ wl_display_dispatch_queue(struct wl_display *display,
>  		return -1;
>  	}
>  
> -	pthread_mutex_lock(&display->mutex);
> -
> -	if (read_events(display) == -1)
> -		goto err_unlock;
> -
> -	ret = dispatch_queue(display, queue);
> -	if (ret == -1)
> -		goto err_unlock;
> -
> -	pthread_mutex_unlock(&display->mutex);
> -
> -	return ret;
> +	if (wl_display_read_events(display) == -1)
> +		return -1;
>  
> - err_unlock:
> -	pthread_mutex_unlock(&display->mutex);
> -	return -1;
> +	return wl_display_dispatch_queue_pending(display, queue);
>  }
>  
>  /** Dispatch pending events in an event queue
> -- 
> 2.4.3
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel