<div dir="ltr"><div>Thanks for reviewing :) English is my weakness (not the only though :D). Fixed patch is coming... </div><div>Marek </div><div><div><div class="gmail_extra"> <div class="gmail_quote">On 19 June 2014 04:24, Bryce W. Harrington <<a href="mailto:b.harrington@samsung.com" target="_blank">b.harrington@samsung.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">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 <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> for pointing out > issues in the first versions of this patch. > > Reviewed-by: Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> </div>Some copyedits below. Otherwise LGTM. Reviewed-by: Bryce Harrington <<a href="mailto:b.harrington@samsung.com">b.harrington@samsung.com</a>> <div><div class="h5"> > --- > 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. </div></div>indicates "in some object an error occurred" might be grammatically better. <div class=""> > + * Difference between this function and display_fatal_error() > + * is that this one handles errors that will come in wire, whereas </div>"come by wire" or "come in by wire" might be clearer <div><div class="h5"> > + * 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-><a href="http://protocol_error.id" target="_blank">protocol_error.id</a> = 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-><a href="http://object.id" target="_blank">object.id</a>, 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-><a href="http://object.id" target="_blank">object.id</a>, > + 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. </div></div>+ * 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. <div><div class="h5">+ * \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-><a href="http://protocol_error.id" target="_blank">protocol_error.id</a>; > + > + 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 > </div></div>> _______________________________________________ > wayland-devel mailing list > <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a></blockquote></div> </div></div></div></div>