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

[Pekka Paalanen]

On Tue, 20 Nov 2018 18:02:50 +1100
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(+)

Looking good!

Pushed:
   921d054..d325140  master -> master


Thanks,
pq


> 
> 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)
>  {

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20190129/57047d5f/attachment.sig>