[PATCH wayland 1/2] protocol: document error cases with wl_shm_pool.create_buffer

[Pekka Paalanen]

On Fri, 30 May 2014 12:08:15 +0200
Jonny Lamb <jonny.lamb at collabora.co.uk> wrote:

> This already happens in weston.
> ---
>  protocol/wayland.xml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> index 22eb6e7..3091d83 100644
> --- a/protocol/wayland.xml
> +++ b/protocol/wayland.xml
> @@ -222,6 +222,11 @@
>  	A buffer will keep a reference to the pool it was created from
>  	so it is valid to destroy the pool immediately after creating
>  	a buffer from it.
> +
> +	If offset is negative, width or height are not greater than
> +	zero, or the stride is less than the width the invalid_stride
> +	protocol error is raised. If the format is not supported the
> +	invalid_format protocol error is raised.
>        </description>
>  
>        <arg name="id" type="new_id" interface="wl_buffer"/>

Hi,

yeah, this is the intention, but the existing protocol spec is strange.

This error happens on wl_shm_pool.create_buffer request, it is sent
against the wl_shm_pool object, but wl_shm_pool does not define this
error!

Instead, the error is defined on wl_shm interface, and technically
those are not valid for wl_shm_pool. The namespace for the error codes
is wrong.

This problem is in both shm_pool_create_buffer() and shm_pool_resize()
functions in libwayland-server.

Furthermore, it seems wl_shm_buffer_end_access() is sending
WL_SHM_ERROR_INVALID_FD on the wl_buffer, which is again a namespace
violation.

Because every interface defines (or does not define) their own error
codes, and these codes are namespaced to the interface (so far all
error enums start with 0 and overlap, I think), the namespacing is
really needed to tell them apart in the receiving side.

So far we haven't had problems, because no-one expects to get a
specific error as part of normal flow. I still think this is a bad
example, and should be fixed.

Proposal:

- Let's duplicate all wl_shm error codes in wl_shm_pool; this maintains
  the numeric values of the codes in a backward-compatible way, and
  makes the namespace valid. Could even add a comment there, that these
  error codes match wl_shm error codes, and wl_shm error codes can be
  used here.

- wl_shm_buffer_end_access() should either use a generic wl_display
  error, or we need to add an error code to wl_buffer interface to have
  the proper namespace. Which?

There is also a practical reason to get this fixed, rather than "oh
it's just ugly": if someone writes a protocol analyzer, that could then
decipher the error codes in addition to showing the error message.


Thanks,
pq