[PATCH] client: extend error handling

Bryce W. Harrington b.harrington at samsung.com
Wed Jun 18 19:24:43 PDT 2014


On Mon, May 26, 2014 at 04:57:23PM +0200, Marek Chalupa wrote:
> When an error occurs, wl_display_get_error() does not
> provide any way of getting know if it was a local error or if it was
> an error event, respectively what object caused the error and what
> the error was.
> 
> This patch introduces a new function wl_display_get_protocol_error()
> which will return error code, interface and id of the object that
> generated the error.
> wl_display_get_error() will work the same way as before.
> 
> wl_display_get_protocol_error() DOES NOT indicate that a non-protocol
> error happened. It returns valid information only in that case that
> (protocol) error occurred, so it should be used after calling
> wl_display_get_error() with positive result.
> 
> Thanks to Pekka Paalanen <ppaalanen at gmail.com> for pointing out
> issues in the first versions of this patch.
> 
> Reviewed-by: Pekka Paalanen <ppaalanen at gmail.com>

Some copyedits below.  Otherwise LGTM.

Reviewed-by: Bryce Harrington <b.harrington at samsung.com>

> ---
>  src/wayland-client.c | 138 ++++++++++++++++++++++++++++++++++++++++++++-------
>  src/wayland-client.h |   3 ++
>  2 files changed, 123 insertions(+), 18 deletions(-)
> 
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index e8aab7e..ee2215d 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -78,7 +78,24 @@ struct wl_event_queue {
>  struct wl_display {
>  	struct wl_proxy proxy;
>  	struct wl_connection *connection;
> +
> +	/* errno of the last wl_display error */
>  	int last_error;
> +
> +	/* When display gets an error event from some object, it stores
> +	 * information about it here, so that client can get this
> +	 * information afterwards */
> +	struct {
> +		/* Code of the error. It can be compared to
> +		 * the interface's errors enumeration. */
> +		uint32_t code;
> +		/* interface (protocol) in which the error occurred */
> +		const struct wl_interface *interface;
> +		/* id of the proxy that caused the error. There's no warranty
> +		 * that the proxy is still valid. It's up to client how it will
> +		 * use it */
> +		uint32_t id;
> +	} protocol_error;
>  	int fd;
>  	pthread_t display_thread;
>  	struct wl_map objects;
> @@ -96,6 +113,14 @@ struct wl_display {
>  
>  static int debug_client = 0;
>  
> +/**
> + * This function is called for local errors (no memory, server hung up)
> + *
> + * \param display
> + * \param error    error value (EINVAL, EFAULT, ...)
> + *
> + * \note this function is called with display mutex locked
> + */
>  static void
>  display_fatal_error(struct wl_display *display, int error)
>  {
> @@ -105,7 +130,7 @@ display_fatal_error(struct wl_display *display, int error)
>  		return;
>  
>  	if (!error)
> -		error = 1;
> +		error = EFAULT;
>  
>  	display->last_error = error;
>  
> @@ -113,11 +138,56 @@ display_fatal_error(struct wl_display *display, int error)
>  		pthread_cond_broadcast(&iter->cond);
>  }
>  
> +/**
> + * This function is called for error events
> + * and idicates that in some object occured an error.

indicates

"in some object an error occurred" might be grammatically better.

> + * Difference between this function and display_fatal_error()
> + * is that this one handles errors that will come in wire, whereas

"come by wire" or "come in by wire" might be clearer

> + * display_fatal_error() is called for local errors.
> + *
> + * \param display
> + * \param code    error code
> + * \param id      id of the object that generated the error
> + * \param intf    protocol interface
> + */
>  static void
> -wl_display_fatal_error(struct wl_display *display, int error)
> +display_protocol_error(struct wl_display *display, uint32_t code,
> +		       uint32_t id, const struct wl_interface *intf)
>  {
> +	struct wl_event_queue *iter;
> +	int err;
> +
> +	if (display->last_error)
> +		return;
> +
> +	/* set correct errno */
> +	if (wl_interface_equal(intf, &wl_display_interface)) {
> +		switch (code) {
> +		case WL_DISPLAY_ERROR_INVALID_OBJECT:
> +		case WL_DISPLAY_ERROR_INVALID_METHOD:
> +			err = EINVAL;
> +			break;
> +		case WL_DISPLAY_ERROR_NO_MEMORY:
> +			err = ENOMEM;
> +			break;
> +		default:
> +			err = EFAULT;
> +		}
> +	} else {
> +		err = EPROTO;
> +	}
> +
>  	pthread_mutex_lock(&display->mutex);
> -	display_fatal_error(display, error);
> +
> +	display->last_error = err;
> +
> +	display->protocol_error.code = code;
> +	display->protocol_error.id = id;
> +	display->protocol_error.interface = intf;
> +
> +	wl_list_for_each(iter, &display->event_queue_list, link)
> +		pthread_cond_broadcast(&iter->cond);
> +
>  	pthread_mutex_unlock(&display->mutex);
>  }
>  
> @@ -579,25 +649,12 @@ display_handle_error(void *data,
>  		     uint32_t code, const char *message)
>  {
>  	struct wl_proxy *proxy = object;
> -	int err;
>  
>  	wl_log("%s@%u: error %d: %s\n",
>  	       proxy->object.interface->name, proxy->object.id, code, message);
>  
> -	switch (code) {
> -	case WL_DISPLAY_ERROR_INVALID_OBJECT:
> -	case WL_DISPLAY_ERROR_INVALID_METHOD:
> -		err = EINVAL;
> -		break;
> -	case WL_DISPLAY_ERROR_NO_MEMORY:
> -		err = ENOMEM;
> -		break;
> -	default:
> -		err = EFAULT;
> -		break;
> -	}
> -
> -	wl_display_fatal_error(display, err);
> +	display_protocol_error(display, code, proxy->object.id,
> +			       proxy->object.interface);
>  }
>  
>  static void
> @@ -1486,6 +1543,51 @@ wl_display_get_error(struct wl_display *display)
>  	return ret;
>  }
>  
> +/**
> + * Retrieve the information about a protocol error
> + *
> + * \param display    display
> + * \param interface  if not NULL, store there an interface on which the error
> + *                   occured
> + * \param id         if not NULL, store there the id of the object that generated
> + *                   the error. There's no warranty that the object is still valid.
> + *                   Client must know if he deleted the object or not.
> + * \return           The error code as defined in the interface specification.


+ * Retrieves the information about a protocol error:
+ *
+ * \param display    The Wayland display
+ * \param interface  if not NULL, stores the interface where the error occurred
+ * \param id         if not NULL, stores the object id that generated
+ *                   the error. There's no guarantee the object is
+ *                   still valid; the client must know if it deleted the object.
+ * \return           The error code as defined in the interface specification.


> + *
> + * \code
> + * int err = wl_display_get_error(display);
> + *
> + * if (err == EPROTO) {
> + *        code = wl_display_get_protocol_error(display, &interface, &id);
> + *        handle_error(code, interface, id);
> + * }
> + *
> + * ...
> + *
> + *  \endcode
> + */
> +WL_EXPORT uint32_t
> +wl_display_get_protocol_error(struct wl_display *display,
> +			      const struct wl_interface **interface,
> +			      uint32_t *id)
> +{
> +	uint32_t ret;
> +
> +	pthread_mutex_lock(&display->mutex);
> +
> +	ret = display->protocol_error.code;
> +
> +	if (interface)
> +		*interface = display->protocol_error.interface;
> +	if (id)
> +		*id = display->protocol_error.id;
> +
> +	pthread_mutex_unlock(&display->mutex);
> +
> +	return ret;
> +}
> +
> +
>  /** Send all buffered requests on the display to the server
>   *
>   * \param display The display context object
> diff --git a/src/wayland-client.h b/src/wayland-client.h
> index 2a32785..dc4df53 100644
> --- a/src/wayland-client.h
> +++ b/src/wayland-client.h
> @@ -161,6 +161,9 @@ int wl_display_dispatch_queue_pending(struct wl_display *display,
>  				      struct wl_event_queue *queue);
>  int wl_display_dispatch_pending(struct wl_display *display);
>  int wl_display_get_error(struct wl_display *display);
> +uint32_t wl_display_get_protocol_error(struct wl_display *display,
> +				       const struct wl_interface **interface,
> +				       uint32_t *id);
>  
>  int wl_display_flush(struct wl_display *display);
>  int wl_display_roundtrip(struct wl_display *display);
> -- 
> 1.9.1
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel


More information about the wayland-devel mailing list