<div dir="ltr">Sorry for not looking at it. I'm mostly ok with the API so it's fine that you pushed it. I did have a question or two though. <div class="gmail_extra"> <div class="gmail_quote">On Mon, Jul 7, 2014 at 7:28 AM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From: Marek Chalupa <<a href="mailto:mchqwerty@gmail.com">mchqwerty@gmail.com</a>> 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. [Pekka Paalanen] Applied another hunk of Bryce's comments to docs, added libtool version bump. Reviewed-by: Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> Reviewed-by: Bryce Harrington <<a href="mailto:b.harrington@samsung.com">b.harrington@samsung.com</a>> --- Makefile.am | 2 +- src/wayland-client.c | 137 ++++++++++++++++++++++++++++++++++++++++++++------- src/wayland-client.h | 3 ++ 3 files changed, 123 insertions(+), 19 deletions(-) diff --git a/Makefile.am b/Makefile.am index c15d8b8..fee19ab 100644 --- a/Makefile.am +++ b/Makefile.am @@ -51,7 +51,7 @@ nodist_libwayland_server_la_SOURCES = \ libwayland_client_la_CFLAGS = $(FFI_CFLAGS) $(GCC_CFLAGS) -pthread libwayland_client_la_LIBADD = $(FFI_LIBS) <a href="http://libwayland-util.la" target="_blank">libwayland-util.la</a> -lrt -lm -libwayland_client_la_LDFLAGS = -version-info 2:0:2 +libwayland_client_la_LDFLAGS = -version-info 3:0:3 libwayland_client_la_SOURCES = \ src/wayland-client.c diff --git a/src/wayland-client.c b/src/wayland-client.c index e8aab7e..19b5694 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 indicates that in some object an error occured. + * Difference between this function and display_fatal_error() + * is that this one handles errors that will come by 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, 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); </blockquote><div> </div><div>Why are you doing this? Is someone waiting on you to display an error or waiting for the error code? I don't really see what there is to wake up here. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + 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,50 @@ wl_display_get_error(struct wl_display *display) return ret; } +/** + * 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. </blockquote><div> </div><div>Does the client even know what to do with the ID? I mean, sure, they could print it in an error message, but how useful is it? I suppose they could check to see if wl_proxy_get_id(my_proxy) == id to see if it happend at that point in the stream. Is this what you intend? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + * \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.8.5.5 _______________________________________________ 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>