[PATCH 2/2] proto, server: Add internal server error message. (v2)

[Christopher James Halse Rogers]

On 20 November 2018 8:02:50 pm NZDT, Christopher James Halse Rogers <christopher.halse.rogers at canonical.com> wrote:
>Many languages such as C++ or Rust have an unwinding error-reporting
>mechanism. Code in these languages can (and must!) wrap request
>handling
>callbacks in unwind guards to avoid undefined behaviour.
>
>As a consequence such code will detect internal server errors, but have
>no way to communicate such failures to the client.
>
>This adds a WL_DISPLAY_ERROR_IMPLEMENTATION error to wl_display so that
>such code can notify (and disconnect) clients which hit internal bugs.
>While servers can currently abuse other wl_display errors for the same
>effect, adding an explicit error code allows clients to tell the
>difference between errors which are their fault and errors which are
>the
>server's fault. This is particularly interesting for automated bug
>reporting.
>
>v2: Rename error from "internal" to "implementation", in sympathy with
>    X11's BadImplementation error.
>    Add more justification in the commit message.
>
>Signed-off-by: Christopher James Halse Rogers
><christopher.halse.rogers at canonical.com>
>Acked-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>---
> protocol/wayland.xml      |  2 ++
> src/wayland-client.c      |  3 +++
> src/wayland-server-core.h |  4 ++++
> src/wayland-server.c      | 24 +++++++++++++++++++++++
> tests/display-test.c      | 40 +++++++++++++++++++++++++++++++++++++++
> 5 files changed, 73 insertions(+)
>
>diff --git a/protocol/wayland.xml b/protocol/wayland.xml
>index 141038b..754b789 100644
>--- a/protocol/wayland.xml
>+++ b/protocol/wayland.xml
>@@ -94,6 +94,8 @@
> 	     summary="method doesn't exist on the specified interface"/>
>       <entry name="no_memory" value="2"
> 	     summary="server is out of memory"/>
>+      <entry name="implementation" value="3"
>+	     summary="implementation error in compositor"/>
>     </enum>
> 
>     <event name="delete_id">
>diff --git a/src/wayland-client.c b/src/wayland-client.c
>index 0ccfc66..b0805f1 100644
>--- a/src/wayland-client.c
>+++ b/src/wayland-client.c
>@@ -185,6 +185,9 @@ display_protocol_error(struct wl_display *display,
>uint32_t code,
> 		case WL_DISPLAY_ERROR_NO_MEMORY:
> 			err = ENOMEM;
> 			break;
>+		case WL_DISPLAY_ERROR_IMPLEMENTATION:
>+			err = EPROTO;
>+			break;
> 		default:
> 			err = EFAULT;
> 		}
>diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
>index 2e725d9..3e0272b 100644
>--- a/src/wayland-server-core.h
>+++ b/src/wayland-server-core.h
>@@ -324,6 +324,10 @@ wl_client_get_object(struct wl_client *client,
>uint32_t id);
> void
> wl_client_post_no_memory(struct wl_client *client);
> 
>+void
>+wl_client_post_implementation_error(struct wl_client *client,
>+                                    const char* msg, ...)
>WL_PRINTF(2,3);
>+
> void
> wl_client_add_resource_created_listener(struct wl_client *client,
>                                         struct wl_listener *listener);
>diff --git a/src/wayland-server.c b/src/wayland-server.c
>index c0ad229..19f6a76 100644
>--- a/src/wayland-server.c
>+++ b/src/wayland-server.c
>@@ -650,6 +650,30 @@ wl_client_post_no_memory(struct wl_client *client)
> 			       WL_DISPLAY_ERROR_NO_MEMORY, "no memory");
> }
> 
>+/** Report an internal server error
>+ *
>+ * \param client The client object
>+ * \param msg A printf-style format string
>+ * \param ... Format string arguments
>+ *
>+ * Report an unspecified internal implementation error and disconnect
>+ * the client.
>+ *
>+ * \memberof wl_client
>+ */
>+WL_EXPORT void
>+wl_client_post_implementation_error(struct wl_client *client,
>+				    char const *msg, ...)
>+{
>+	va_list ap;
>+
>+	va_start(ap, msg);
>+	wl_resource_post_error_vargs(client->display_resource,
>+				     WL_DISPLAY_ERROR_IMPLEMENTATION,
>+				     msg, ap);
>+	va_end(ap);
>+}
>+
> WL_EXPORT void
> wl_resource_post_no_memory(struct wl_resource *resource)
> {
>diff --git a/tests/display-test.c b/tests/display-test.c
>index 9b49a0e..6d98cc7 100644
>--- a/tests/display-test.c
>+++ b/tests/display-test.c
>@@ -419,6 +419,46 @@ TEST(post_nomem_tst)
> 	display_destroy(d);
> }
> 
>+static void
>+post_implementation_error_main(void)
>+{
>+	struct client *c = client_connect();
>+	struct wl_seat *seat = client_get_seat(c);
>+	uint32_t object_id, protocol_error;
>+	const struct wl_interface *interface;
>+
>+	assert(stop_display(c, 1) == -1);
>+	int err = wl_display_get_error(c->wl_display);
>+	fprintf(stderr, "Err is %i\n", err);
>+	assert(err == EPROTO);
>+	protocol_error = wl_display_get_protocol_error(c->wl_display,
>+						       &interface,
>+						       &object_id);
>+	assert(protocol_error == WL_DISPLAY_ERROR_IMPLEMENTATION);
>+	assert(interface == &wl_display_interface);
>+
>+	wl_proxy_destroy((struct wl_proxy *) seat);
>+	client_disconnect_nocheck(c);
>+}
>+
>+TEST(post_internal_error_tst)
>+{
>+	struct display *d = display_create();
>+	struct client_info *cl;
>+
>+	wl_global_create(d->wl_display, &wl_seat_interface,
>+			 1, d, bind_seat);
>+
>+	cl = client_create_noarg(d, post_implementation_error_main);
>+	display_run(d);
>+
>+	wl_client_post_implementation_error(cl->wl_client, "Error %i", 20);
>+
>+	display_resume(d);
>+
>+	display_destroy(d);
>+}
>+
> static void
> register_reading(struct wl_display *display)
> {
>-- 
>2.19.1
>
>_______________________________________________
>wayland-devel mailing list
>wayland-devel at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Ping!

Is there anything I can do to make these patches more appealing?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20190122/8a9afcd2/attachment.html>