<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On 5 May 2014 15:02, Pekka Paalanen <span dir="ltr"><<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wed, 30 Apr 2014 10:11:01 +0200<br>
Marek Chalupa <<a href="mailto:mchqwerty@gmail.com">mchqwerty@gmail.com</a>> wrote:<br>
<br>
> When an error occurs, wl_display_get_error() does not<br>
> provide any way of getting know if it was a local error or if it was<br>
> an error event, respectively what object caused the error and what<br>
> the error was.<br>
><br>
> This patch introduces a new function wl_display_get_protocol_error()<br>
> which will return error code, interface and id of the object that<br>
> generated the error.<br>
> wl_display_get_error() will work the same way as before.<br>
><br>
> wl_display_get_protocol_error() DOES NOT indicate that a non-protocol<br>
> error happened. It returns valid information only in that case that<br>
> (protocol) error occurred, so it should be used after calling<br>
> wl_display_get_error() with positive result.<br>
><br>
> Thanks to Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> for pointing out<br>
> issues in the first versions of this patch.<br>
> ---<br>
>  src/wayland-client.c | 138 ++++++++++++++++++++++++++++++++++++++++++++-------<br>
>  src/wayland-client.h |   3 ++<br>
>  2 files changed, 123 insertions(+), 18 deletions(-)<br>
><br>
> diff --git a/src/wayland-client.c b/src/wayland-client.c<br>
> index bd40313..8dd8804 100644<br>
> --- a/src/wayland-client.c<br>
> +++ b/src/wayland-client.c<br>
> @@ -78,7 +78,24 @@ struct wl_event_queue {<br>
>  struct wl_display {<br>
>       struct wl_proxy proxy;<br>
>       struct wl_connection *connection;<br>
> +<br>
> +     /* errno of the last wl_display error */<br>
>       int last_error;<br>
> +<br>
> +     /* When display gets an error event from some object, it stores<br>
> +      * information about it here, so that client can get this<br>
> +      * information afterwards */<br>
> +     struct {<br>
> +             /* Code of the error. It can be compared to<br>
> +              * the interface's errors enumeration. */<br>
> +             int code;<br>
<br>
</div></div>I just checked in wl_display::error specification, and the type of the<br>
error code is uint32_t.<br>
<div><div class="h5"><br>
> +             /* interface (protocol) in which the error occurred */<br>
> +             const struct wl_interface *interface;<br>
> +             /* id of the proxy that caused the error. There's no warranty<br>
> +              * that the proxy is still valid. It's up to client how it will<br>
> +              * use it */<br>
> +             uint32_t id;<br>
> +     } protocol_error;<br>
>       int fd;<br>
>       pthread_t display_thread;<br>
>       struct wl_map objects;<br>
> @@ -96,6 +113,14 @@ struct wl_display {<br>
><br>
>  static int debug_client = 0;<br>
><br>
> +/**<br>
> + * This function is called for local errors (no memory, server hung up)<br>
> + *<br>
> + * \param display<br>
> + * \param error    error value (EINVAL, EFAULT, ...)<br>
> + *<br>
> + * \note this function is called with display mutex locked<br>
> + */<br>
>  static void<br>
>  display_fatal_error(struct wl_display *display, int error)<br>
>  {<br>
> @@ -105,7 +130,7 @@ display_fatal_error(struct wl_display *display, int error)<br>
>               return;<br>
><br>
>       if (!error)<br>
> -             error = 1;<br>
> +             error = EFAULT;<br>
><br>
>       display->last_error = error;<br>
><br>
> @@ -113,11 +138,56 @@ display_fatal_error(struct wl_display *display, int error)<br>
>               pthread_cond_broadcast(&iter->cond);<br>
>  }<br>
><br>
> +/**<br>
> + * This function is called for error events<br>
> + * and idicates that in some object occured an error.<br>
> + * Difference between this function and display_fatal_error()<br>
> + * is that this one handles errors that will come in wire, whereas<br>
> + * display_fatal_error() is called for local errors.<br>
> + *<br>
> + * \param display<br>
> + * \param code    error code<br>
> + * \param id      id of the object that generated the error<br>
> + * \param intf    protocol interface<br>
> + */<br>
>  static void<br>
> -wl_display_fatal_error(struct wl_display *display, int error)<br>
> +display_protocol_error(struct wl_display *display, int code,<br>
> +                    unsigned int id, const struct wl_interface *intf)<br>
>  {<br>
> +     struct wl_event_queue *iter;<br>
> +     int err;<br>
> +<br>
> +     if (display->last_error)<br>
> +             return;<br>
> +<br>
> +     /* set correct errno */<br>
> +     if (wl_interface_equal(intf, &wl_display_interface)) {<br>
> +             switch (code) {<br>
> +             case WL_DISPLAY_ERROR_INVALID_OBJECT:<br>
> +             case WL_DISPLAY_ERROR_INVALID_METHOD:<br>
> +                     err = EINVAL;<br>
> +                     break;<br>
> +             case WL_DISPLAY_ERROR_NO_MEMORY:<br>
> +                     err = ENOMEM;<br>
> +                     break;<br>
> +             default:<br>
> +                     err = EFAULT;<br>
> +             }<br>
> +     } else {<br>
> +             err = EPROTO;<br>
> +     }<br>
> +<br>
>       pthread_mutex_lock(&display->mutex);<br>
> -     display_fatal_error(display, error);<br>
> +<br>
> +     display->last_error = err;<br>
> +<br>
> +     display->protocol_error.code = code;<br>
> +     display-><a href="http://protocol_error.id" target="_blank">protocol_error.id</a> = id;<br>
> +     display->protocol_error.interface = intf;<br>
> +<br>
> +     wl_list_for_each(iter, &display->event_queue_list, link)<br>
> +             pthread_cond_broadcast(&iter->cond);<br>
> +<br>
>       pthread_mutex_unlock(&display->mutex);<br>
>  }<br>
><br>
> @@ -579,25 +649,12 @@ display_handle_error(void *data,<br>
>                    uint32_t code, const char *message)<br>
>  {<br>
>       struct wl_proxy *proxy = object;<br>
> -     int err;<br>
><br>
>       wl_log("%s@%u: error %d: %s\n",<br>
>              proxy->object.interface->name, proxy-><a href="http://object.id" target="_blank">object.id</a>, code, message);<br>
><br>
> -     switch (code) {<br>
> -     case WL_DISPLAY_ERROR_INVALID_OBJECT:<br>
> -     case WL_DISPLAY_ERROR_INVALID_METHOD:<br>
> -             err = EINVAL;<br>
> -             break;<br>
> -     case WL_DISPLAY_ERROR_NO_MEMORY:<br>
> -             err = ENOMEM;<br>
> -             break;<br>
> -     default:<br>
> -             err = EFAULT;<br>
> -             break;<br>
> -     }<br>
> -<br>
> -     wl_display_fatal_error(display, err);<br>
> +     display_protocol_error(display, code, proxy-><a href="http://object.id" target="_blank">object.id</a>,<br>
> +                            proxy->object.interface);<br>
>  }<br>
><br>
>  static void<br>
> @@ -1489,6 +1546,51 @@ wl_display_get_error(struct wl_display *display)<br>
>       return ret;<br>
>  }<br>
><br>
> +/**<br>
> + * Retrieve the information about a protocol error<br>
> + *<br>
> + * \param display    display<br>
> + * \param interface  if not NULL, store there an interface on which the error<br>
> + *                   occured<br>
> + * \param id         if not NULL, store there the id of the object that generated<br>
> + *                   the error. There's no warranty that the object is still valid.<br>
> + *                   Client must know if he deleted the object or not.<br>
> + * \return           The error code as defined in the interface specification.<br>
> + *<br>
> + * \code<br>
> + * int err = wl_display_get_error(display);<br>
> + *<br>
> + * if (err == EPROTO) {<br>
> + *        code = wl_display_get_protocol_error(display, &interface, &id);<br>
> + *        handle_error(code, interface, id);<br>
> + * }<br>
> + *<br>
> + * ...<br>
> + *<br>
> + *  \endcode<br>
> + */<br>
> +WL_EXPORT int<br>
<br>
</div></div>Needs to be uint32_t.<br>
<div><div class="h5"><br>
> +wl_display_get_protocol_error(struct wl_display *display,<br>
> +                           const struct wl_interface **interface,<br>
> +                           uint32_t *id)<br>
> +{<br>
> +     int ret;<br>
> +<br>
> +     pthread_mutex_lock(&display->mutex);<br>
> +<br>
> +     ret = display->protocol_error.code;<br>
> +<br>
> +     if (interface)<br>
> +             *interface = display->protocol_error.interface;<br>
> +     if (id)<br>
> +             *id = display-><a href="http://protocol_error.id" target="_blank">protocol_error.id</a>;<br>
> +<br>
> +     pthread_mutex_unlock(&display->mutex);<br>
> +<br>
> +     return ret;<br>
> +}<br>
> +<br>
> +<br>
>  /** Send all buffered requests on the display to the server<br>
>   *<br>
>   * \param display The display context object<br>
> diff --git a/src/wayland-client.h b/src/wayland-client.h<br>
> index 2a32785..99fac06 100644<br>
> --- a/src/wayland-client.h<br>
> +++ b/src/wayland-client.h<br>
> @@ -161,6 +161,9 @@ int wl_display_dispatch_queue_pending(struct wl_display *display,<br>
>                                     struct wl_event_queue *queue);<br>
>  int wl_display_dispatch_pending(struct wl_display *display);<br>
>  int wl_display_get_error(struct wl_display *display);<br>
> +int wl_display_get_protocol_error(struct wl_display *display,<br>
<br>
</div></div>uint32_t.<br>
<div class=""><br>
> +                               const struct wl_interface **interface,<br>
> +                               uint32_t *id);<br>
><br>
>  int wl_display_flush(struct wl_display *display);<br>
>  int wl_display_roundtrip(struct wl_display *display);<br>
<br>
</div>Hi,<br>
<br>
sorry for not noticing earlier that the wl_display::error event's code<br>
is uint32_t.</blockquote><div><br></div><div>Well, I should have checked that. Thanks :)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">With that fixed, this patch is<br>

Reviewed-by: Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>><br>
<br>
The next step would be to add a test in libwayland to excercise this. I<br>
took a quick look but I'm not sure which one would be the best example<br>
to derive from.<br>
 <br></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks,<br>
pq<br>
</blockquote></div><br></div><div class="gmail_extra">New (and hopefully the last) version of the patch is cooming.<br><br></div><div class="gmail_extra">Thanks,<br></div><div class="gmail_extra">Marek<br></div></div>