On 5 May 2014 15:02, Pekka Paalanen <ppaala...@gmail.com> wrote: > On Wed, 30 Apr 2014 10:11:01 +0200 > Marek Chalupa <mchqwe...@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 <ppaala...@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.
Well, I should have checked that. Thanks :) > With that fixed, this patch is > Reviewed-by: Pekka Paalanen <ppaala...@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 > New (and hopefully the last) version of the patch is cooming. Thanks, Marek
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel