Re: [PATCH v3] client: extend error handling
On Wed, 7 May 2014 16:21:24 +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 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..c9f8f4f 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 errorerror 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 codeerror code
> + * \param id id of the object that generated the error
> + * \param intfprotocol 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
Re: [PATCH v3] client: extend error handling
On 5 May 2014 15:02, Pekka Paalanen wrote:
> On Wed, 30 Apr 2014 10:11:01 +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 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 errorerror 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 codeerror code
> > + * \param id id of the object that generated the error
> > + * \param intfprotocol 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
Re: [PATCH v3] client: extend error handling
On Wed, 30 Apr 2014 10:11:01 +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 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 errorerror 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 codeerror code
> + * \param id id of the object that generated the error
> + * \param intfprotocol 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 *mes
