[PATCH v3] client: extend error handling
Pekka Paalanen
ppaalanen at gmail.com
Mon May 5 06:02:22 PDT 2014
On Wed, 30 Apr 2014 10:11:01 +0200
Marek Chalupa <mchqwerty at gmail.com> 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.
> ---
> 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 bd40313..8dd8804 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. */
> + int code;
I just checked in wl_display::error specification, and the type of the
error code is uint32_t.
> + /* 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.
> + * Difference between this function and display_fatal_error()
> + * is that this one handles errors that will come in wire, whereas
> + * 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, int code,
> + unsigned int 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
> @@ -1489,6 +1546,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.
> + *
> + * \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 int
Needs to be uint32_t.
> +wl_display_get_protocol_error(struct wl_display *display,
> + const struct wl_interface **interface,
> + uint32_t *id)
> +{
> + int 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..99fac06 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);
> +int wl_display_get_protocol_error(struct wl_display *display,
uint32_t.
> + const struct wl_interface **interface,
> + uint32_t *id);
>
> int wl_display_flush(struct wl_display *display);
> int wl_display_roundtrip(struct wl_display *display);
Hi,
sorry for not noticing earlier that the wl_display::error event's code
is uint32_t. With that fixed, this patch is
Reviewed-by: Pekka Paalanen <ppaalanen at gmail.com>
The next step would be to add a test in libwayland to excercise this. I
took a quick look but I'm not sure which one would be the best example
to derive from.
Thanks,
pq
More information about the wayland-devel
mailing list