[PATCH v2 wayland] client: Allow absolute paths in WAYLAND_DISPLAY

Tue Nov 14 14:22:22 UTC 2017

On Fri, 10 Nov 2017 11:20:42 -0600
Matt Hoosier <matt.hoosier at gmail.com> wrote:

> From: Matt Hoosier <matt.hoosier at garmin.com>
> 
> In order to support system compositor instances, it is necessary to
> allow clients' wl_display_connect() to find the compositor's listening
> socket somewhere outside of XDG_RUNTIME_DIR. For a full account, see
> the discussion beginning here:
> 
> https://lists.freedesktop.org/archives/wayland-devel/2017-November/035664.html
> 
> This change adjusts the client-side connection logic so that, if
> WAYLAND_DISPLAY is formatted as an absolute pathname, the socket
> connection attempt is made to just $WAYLAND_DISPLAY rather than
> usual user-private location $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY.
> 
> This change is based on Davide Bettio's submission of the same concept
> at:
> 
> https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html.
> 
> v2 changes:
> 
> * Added backward incompatibility note to wl_display_connect() manpage.
> * Rephased wl_display_connect() manpage changes to precisely match actual
>   changed behavior.
> * Added mention of new absolute path behavior in wl_display_connect()
>   doxygen comments.
> * Mentioned new absolute path interpretation of WAYLAND_DISPLAY in
>   protocol documentation.
> 
> Signed-off-by: Matt Hoosier <matt.hoosier at gmail.com>
> ---
>  doc/man/wl_display_connect.xml    | 22 +++++++++++++++++---
>  doc/publican/sources/Protocol.xml |  5 ++++-
>  src/wayland-client.c              | 43 +++++++++++++++++++++++++++++----------
>  3 files changed, 55 insertions(+), 15 deletions(-)
> 
> diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml
> index 7e6e05c..7bdfc46 100644
> --- a/doc/man/wl_display_connect.xml
> +++ b/doc/man/wl_display_connect.xml
> @@ -55,14 +55,30 @@
>      <title>Description</title>
>      <para><function>wl_display_connect</function> connects to a Wayland socket
>            that was previously opened by a Wayland server. The server socket must
> -          be placed in <envar>XDG_RUNTIME_DIR</envar> for this function to
> -          find it. The <varname>name</varname> argument specifies the name of
> +          be placed in <envar>XDG_RUNTIME_DIR</envar> or exist at the absolute
> +          path referred to by <envar>WAYLAND_DISPLAY</envar> for this function
> +          to find it. The <varname>name</varname> argument specifies the name of
>            the socket or <constant>NULL</constant> to use the default (which is
>            <constant>"wayland-0"</constant>). The environment variable
>            <envar>WAYLAND_DISPLAY</envar> replaces the default value. If
>            <envar>WAYLAND_SOCKET</envar> is set, this function behaves like
>            <function>wl_display_connect_to_fd</function> with the file-descriptor
> -          number taken from the environment variable.</para>
> +          number taken from the environment variable. If
> +          <envar>WAYLAND_SOCKET</envar> is not set and <varname>name</varname>
> +          is <constant>NULL</constant> and <envar>WAYLAND_DISPLAY</envar>
> +          is an absolute path, then the path stored in <envar>WAYLAND_DISPLAY</envar>
> +          is used as the Wayland socket to which the connection is attempted.</para>
> +
> +    <para>Support for interpreting <envar>WAYLAND_DISPLAY</envar> as an
> +          absolute path is a change in behavior compared to
> +          <function>wl_display_connect</function>'s behavior in versions
> +          1.14 and older of Wayland. It is no longer guaranteed in versions
> +          1.15 and higher that the Wayland socket chosen is equivalent to
> +          manually constructing a socket pathname by concatenating
> +          <envar>XDG_RUNTIME_DIR</envar> and <envar>WAYLAND_DISPLAY</envar>.
> +          Manual construction of the socket path must account for the
> +          possibility that <envar>WAYLAND_DISPLAY</envar> contains an absolute
> +          path.</para>

Hi Matt,

this is a bit verbose, but it is correct. 'name' argument could now be
absolute as well.

>  
>      <para><function>wl_display_connect_to_fd</function> connects to a Wayland
>            socket with an explicit file-descriptor. The file-descriptor is passed
> diff --git a/doc/publican/sources/Protocol.xml b/doc/publican/sources/Protocol.xml
> index ba6b5f1..dbfed06 100644
> --- a/doc/publican/sources/Protocol.xml
> +++ b/doc/publican/sources/Protocol.xml
> @@ -94,7 +94,10 @@
>        The protocol is sent over a UNIX domain stream socket, where the endpoint
>        usually is named <systemitem class="service">wayland-0</systemitem>
>        (although it can be changed via <emphasis>WAYLAND_DISPLAY</emphasis>
> -      in the environment).
> +      in the environment). In the reference implementation, a client whose
> +      <emphasis>WAYLAND_DISPLAY</emphasis> is formatted as an absolute path
> +      connects to that path as the endpoint, otherwise the implementation
> +      searches in <emphasis>XDG_RUNTIME_DIR</emphasis> for the endpoint.
>      </para>
>      <para>
>        Every message is structured as 32-bit words; values are represented in the
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index 3d7361e..d90cbfb 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c

The code here is good.

> @@ -994,6 +1009,12 @@ wl_display_connect_to_fd(int fd)
>   * its value will be replaced with the WAYLAND_DISPLAY environment
>   * variable if it is set, otherwise display "wayland-0" will be used.
>   *
> + * If \c name is \c NULL and the WAYLAND_DISPLAY environment variable
> + * is set to an absolute pathname, then the pathname stored in
> + * WAYLAND_DISPLAY is used immediately as the location of the

s/immediately/as is/?

> + * socket at which the Wayland display is listening; no qualification
> + * inside XDG_RUNTIME_DIR is attempted.

There is one more case: 'name' itself can be an absolute path now.

Would be good to mention the libwayland version where this changed here
as well.

> + *
>   * \memberof wl_display
>   */
>  WL_EXPORT struct wl_display *

Looking pretty good to me, but I'll save my R-b for the revision with
the test.

Thanks,
pq
-------------- 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/20171114/e4e207dd/attachment-0001.sig>