On Tue, 20 Feb 2018 18:07:27 +1100
r...@ubuntu.com wrote:
From: Christopher James Halse Rogers
<christopher.halse.rog...@canonical.com>
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_INTERNAL error to wl_display so that
such code can notify (and disconnect) client which hit internal
bugs.
Signed-off-by: Christopher James Halse Rogers
<christopher.halse.rog...@canonical.com>
---
protocol/wayland.xml | 2 ++
src/wayland-client.c | 3 +++
src/wayland-server-core.h | 4 ++++
src/wayland-server.c | 23 +++++++++++++++++++++++
tests/display-test.c | 40
++++++++++++++++++++++++++++++++++++++++
5 files changed, 72 insertions(+)
diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index b5662e0..1db31a6 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="internal" value="3"
+ summary="internal server error"/>
</enum>
<event name="delete_id">
diff --git a/src/wayland-client.c b/src/wayland-client.c
index c1369b8..7c442b1 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -192,6 +192,9 @@ display_protocol_error(struct wl_display
*display, uint32_t code,
case WL_DISPLAY_ERROR_NO_MEMORY:
err = ENOMEM;
break;
+ case WL_DISPLAY_ERROR_INTERNAL:
+ err = EPROTO;
+ break;
default:
err = EFAULT;
}
diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
index 2e725d9..7137da6 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_internal_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 00c93f7..6317f8f 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -650,6 +650,29 @@ 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 error and disconnect the client.
+ *
+ * \memberof wl_client
+ */
+WL_EXPORT void
+wl_client_post_internal_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_INTERNAL,
+ msg, ap);
+ va_end(ap);
+}
+
Hi,
the explanation here is more explicit than what we discussed in IRC,
and both together give a nice justifiction for adding this. I'm for
it,
and I am very happy to see the tests extended as well.
What we discussed in IRC was around why add a new error code instead
of
abusing the existing wl_display error codes (it is possible to send
wl_display errors explicitly with custom server code). The outcome of
that was that it would be useful for applications to tell the
difference between their fault and server's fault, to guide automated
bug reporting. This would be nice to add to the commit message.
The one thing I'm wondering is, is "internal" obvious enough to say
it's a server internal error and not a (lib)Wayland internal error?
I've seen Wayland blamed so wildly for things it has nothing to do
with
that I might prefer something more explicit, like "implementation"
mentioned by Daniel in IRC. OTOH, what user would see is probably only
the numerical value of the error with the message, so maybe it's
unlikely to be confused.
