[PATCH wayland] Require WAYLAND_DISPLAY to be set instead of using wayland-0 as the default

Pekka Paalanen ppaalanen at gmail.com
Fri Aug 14 00:20:52 PDT 2015


On Wed, 12 Aug 2015 19:34:31 -0700
Dima Ryazanov <dima at gmail.com> wrote:

> Although defaulting to wayland-0 seems convenient, it has an undesirable
> side effect: clients may unintentionally connect to the wrong compositor.
> Generally, it's safer to fail instead. Here's a real example:
> 
> In Fedora 22, Gtk+ prefers Wayland over X11, though the default session is still
> a normal X11 Gnome session. When you launch a Gtk+ app, it will try Wayland,
> fail, then try X11, and succesfully start up. That works fine.
> 
> Now suppose you launch Weston while running the Gnome session. Suddenly, all
> of the Gtk+ apps launched from Gnome will show up inside Weston instead.
> That's unexpected. There's also no good way to prevent that from happening
> (other than perhaps setting WAYLAND_DISPLAY to an invalid value when launching
> an app).
> 
> Not using wayland-0 as the default will solve that problem: an app launched
> from the X11 Gnome session will use the X11 backend regardless of whether
> there's a wayland compositor running at the same time.
> 
> Everything else should work as before. The compositor already sets
> the WAYLAND_DISPLAY when starting the session, so the lack of the default value
> should not make a difference to the user.
> 
> Signed-off-by: Dima Ryazanov <dima at gmail.com>
> Acked-by: Pekka Paalanen <ppaalanen at gmail.com>
> Acked-by: Giulio Camuffo <giuliocamuffo at gmail.com>
> Acked-by: Daniel Stone <daniel at fooishbar.org>
> Acked-by: Jasper St. Pierre <jstpierre at mecheye.net>
> ---
>  doc/man/wl_display_connect.xml    |  5 ++---
>  doc/publican/sources/Protocol.xml |  8 ++++----
>  src/wayland-client.c              | 10 ++++++----
>  src/wayland-server.c              |  6 +++---
>  4 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml
> index 7e6e05c..ded3cbd 100644
> --- a/doc/man/wl_display_connect.xml
> +++ b/doc/man/wl_display_connect.xml
> @@ -57,9 +57,8 @@
>            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
> -          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
> +          the socket or <constant>NULL</constant> to use the default
> +          (which is the value of <envar>WAYLAND_DISPLAY</envar>). 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>
> diff --git a/doc/publican/sources/Protocol.xml b/doc/publican/sources/Protocol.xml
> index 477063b..9464953 100644
> --- a/doc/publican/sources/Protocol.xml
> +++ b/doc/publican/sources/Protocol.xml
> @@ -60,10 +60,10 @@
>      <title>Wire Format</title>
>      <para>
>        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).  The protocol is message-based.  A
> -      message sent by a client to the server is called request.  A message
> +      name is determined by the <emphasis>WAYLAND_DISPLAY</emphasis>
> +      environment variable.  Its value will usually be
> +      <systemitem class="service">wayland-0</systemitem>.  The protocol is message-based.
> +      A message sent by a client to the server is called request.  A message
>        from the server to a client is called event.  Every message is
>        structured as 32-bit words, values are represented in the host's
>        byte-order.
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index 09c594a..ffbca4b 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -764,8 +764,11 @@ connect_to_socket(const char *name)
>  
>  	if (name == NULL)
>  		name = getenv("WAYLAND_DISPLAY");
> -	if (name == NULL)
> -		name = "wayland-0";
> +	if (name == NULL) {
> +		wl_log("error: WAYLAND_DISPLAY not set in the environment.\n");
> +		errno = ENOENT;
> +		return -1;
> +	}
>  
>  	fd = wl_os_socket_cloexec(PF_LOCAL, SOCK_STREAM, 0);
>  	if (fd < 0)
> @@ -869,8 +872,7 @@ wl_display_connect_to_fd(int fd)
>   * \return A \ref wl_display object or \c NULL on failure
>   *
>   * Connect to the Wayland display named \c name. If \c name is \c NULL,
> - * its value will be replaced with the WAYLAND_DISPLAY environment
> - * variable if it is set, otherwise display "wayland-0" will be used.
> + * its value will be replaced with the WAYLAND_DISPLAY environment variable.
>   *
>   * \memberof wl_display
>   */

Hi,

this patch is Reviewed-by me up to this point, but I don't agree with
the below change to wayland-server.c as part of this patch.

> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 0f04f66..1ea53f9 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -1209,8 +1209,8 @@ wl_display_add_socket_auto(struct wl_display *display)
>   * connect to Wayland display.
>   *
>   * If NULL is passed as name, then it would look for WAYLAND_DISPLAY env
> - * variable for the socket name. If WAYLAND_DISPLAY is not set, then default
> - * wayland-0 is used.
> + * variable for the socket name. If WAYLAND_DISPLAY is not set, this function
> + * fails and returns -1.
>   *
>   * The Unix socket will be created in the directory pointed to by environment
>   * variable XDG_RUNTIME_DIR. If XDG_RUNTIME_DIR is not set, then this function
> @@ -1235,7 +1235,7 @@ wl_display_add_socket(struct wl_display *display, const char *name)
>  	if (name == NULL)
>  		name = getenv("WAYLAND_DISPLAY");
>  	if (name == NULL)
> -		name = "wayland-0";
> +		return -1;
>  
>  	if (wl_socket_init_for_display_name(s, name) < 0) {
>  		wl_socket_destroy(s);

wl_display_add_socket() is used by the server to create a socket. If
the program does not give a name parameter, it's probably ok to try and
default to "wayland-0". I am not sure why WAYLAND_DISPLAY is involved
at all.

Anyway, the essence of this patch is to fix libwayland-client, so this
change to libwayland-server is off-topic anyway and should be
considered separately.

I dropped the hunk to wayland-server.c and committed the rest.

Pushed:
   441f9bb..fb7e130  master -> master


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150814/a71dfd24/attachment-0001.sig>


More information about the wayland-devel mailing list