<div dir="ltr"><div><div>Oh, you're right, I didn't realize that's what that code was doing.<br></div><br></div>Thanks!<br><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Aug 14, 2015 at 12:20 AM, Pekka Paalanen <span dir="ltr"><<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed, 12 Aug 2015 19:34:31 -0700<br>
Dima Ryazanov <<a href="mailto:dima@gmail.com">dima@gmail.com</a>> wrote:<br>
<br>
</span><div><div class="h5">> Although defaulting to wayland-0 seems convenient, it has an undesirable<br>
> side effect: clients may unintentionally connect to the wrong compositor.<br>
> Generally, it's safer to fail instead. Here's a real example:<br>
><br>
> In Fedora 22, Gtk+ prefers Wayland over X11, though the default session is still<br>
> a normal X11 Gnome session. When you launch a Gtk+ app, it will try Wayland,<br>
> fail, then try X11, and succesfully start up. That works fine.<br>
><br>
> Now suppose you launch Weston while running the Gnome session. Suddenly, all<br>
> of the Gtk+ apps launched from Gnome will show up inside Weston instead.<br>
> That's unexpected. There's also no good way to prevent that from happening<br>
> (other than perhaps setting WAYLAND_DISPLAY to an invalid value when launching<br>
> an app).<br>
><br>
> Not using wayland-0 as the default will solve that problem: an app launched<br>
> from the X11 Gnome session will use the X11 backend regardless of whether<br>
> there's a wayland compositor running at the same time.<br>
><br>
> Everything else should work as before. The compositor already sets<br>
> the WAYLAND_DISPLAY when starting the session, so the lack of the default value<br>
> should not make a difference to the user.<br>
><br>
> Signed-off-by: Dima Ryazanov <<a href="mailto:dima@gmail.com">dima@gmail.com</a>><br>
> Acked-by: Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>><br>
> Acked-by: Giulio Camuffo <<a href="mailto:giuliocamuffo@gmail.com">giuliocamuffo@gmail.com</a>><br>
> Acked-by: Daniel Stone <<a href="mailto:daniel@fooishbar.org">daniel@fooishbar.org</a>><br>
> Acked-by: Jasper St. Pierre <<a href="mailto:jstpierre@mecheye.net">jstpierre@mecheye.net</a>><br>
> ---<br>
> doc/man/wl_display_connect.xml | 5 ++---<br>
> doc/publican/sources/Protocol.xml | 8 ++++----<br>
> src/wayland-client.c | 10 ++++++----<br>
> src/wayland-server.c | 6 +++---<br>
> 4 files changed, 15 insertions(+), 14 deletions(-)<br>
><br>
> diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml<br>
> index 7e6e05c..ded3cbd 100644<br>
> --- a/doc/man/wl_display_connect.xml<br>
> +++ b/doc/man/wl_display_connect.xml<br>
> @@ -57,9 +57,8 @@<br>
> that was previously opened by a Wayland server. The server socket must<br>
> be placed in <envar>XDG_RUNTIME_DIR</envar> for this function to<br>
> find it. The <varname>name</varname> argument specifies the name of<br>
> - the socket or <constant>NULL</constant> to use the default (which is<br>
> - <constant>"wayland-0"</constant>). The environment variable<br>
> - <envar>WAYLAND_DISPLAY</envar> replaces the default value. If<br>
> + the socket or <constant>NULL</constant> to use the default<br>
> + (which is the value of <envar>WAYLAND_DISPLAY</envar>). If<br>
> <envar>WAYLAND_SOCKET</envar> is set, this function behaves like<br>
> <function>wl_display_connect_to_fd</function> with the file-descriptor<br>
> number taken from the environment variable.</para><br>
> diff --git a/doc/publican/sources/Protocol.xml b/doc/publican/sources/Protocol.xml<br>
> index 477063b..9464953 100644<br>
> --- a/doc/publican/sources/Protocol.xml<br>
> +++ b/doc/publican/sources/Protocol.xml<br>
> @@ -60,10 +60,10 @@<br>
> <title>Wire Format</title><br>
> <para><br>
> The protocol is sent over a UNIX domain stream socket, where the endpoint<br>
> - usually is named <systemitem class="service">wayland-0</systemitem><br>
> - (although it can be changed via <emphasis>WAYLAND_DISPLAY</emphasis><br>
> - in the environment). The protocol is message-based. A<br>
> - message sent by a client to the server is called request. A message<br>
> + name is determined by the <emphasis>WAYLAND_DISPLAY</emphasis><br>
> + environment variable. Its value will usually be<br>
> + <systemitem class="service">wayland-0</systemitem>. The protocol is message-based.<br>
> + A message sent by a client to the server is called request. A message<br>
> from the server to a client is called event. Every message is<br>
> structured as 32-bit words, values are represented in the host's<br>
> byte-order.<br>
> diff --git a/src/wayland-client.c b/src/wayland-client.c<br>
> index 09c594a..ffbca4b 100644<br>
> --- a/src/wayland-client.c<br>
> +++ b/src/wayland-client.c<br>
> @@ -764,8 +764,11 @@ connect_to_socket(const char *name)<br>
><br>
> if (name == NULL)<br>
> name = getenv("WAYLAND_DISPLAY");<br>
> - if (name == NULL)<br>
> - name = "wayland-0";<br>
> + if (name == NULL) {<br>
> + wl_log("error: WAYLAND_DISPLAY not set in the environment.\n");<br>
> + errno = ENOENT;<br>
> + return -1;<br>
> + }<br>
><br>
> fd = wl_os_socket_cloexec(PF_LOCAL, SOCK_STREAM, 0);<br>
> if (fd < 0)<br>
> @@ -869,8 +872,7 @@ wl_display_connect_to_fd(int fd)<br>
> * \return A \ref wl_display object or \c NULL on failure<br>
> *<br>
> * Connect to the Wayland display named \c name. If \c name is \c NULL,<br>
> - * its value will be replaced with the WAYLAND_DISPLAY environment<br>
> - * variable if it is set, otherwise display "wayland-0" will be used.<br>
> + * its value will be replaced with the WAYLAND_DISPLAY environment variable.<br>
> *<br>
> * \memberof wl_display<br>
> */<br>
<br>
</div></div>Hi,<br>
<br>
this patch is Reviewed-by me up to this point, but I don't agree with<br>
the below change to wayland-server.c as part of this patch.<br>
<span class=""><br>
> diff --git a/src/wayland-server.c b/src/wayland-server.c<br>
> index 0f04f66..1ea53f9 100644<br>
> --- a/src/wayland-server.c<br>
> +++ b/src/wayland-server.c<br>
> @@ -1209,8 +1209,8 @@ wl_display_add_socket_auto(struct wl_display *display)<br>
> * connect to Wayland display.<br>
> *<br>
> * If NULL is passed as name, then it would look for WAYLAND_DISPLAY env<br>
> - * variable for the socket name. If WAYLAND_DISPLAY is not set, then default<br>
> - * wayland-0 is used.<br>
> + * variable for the socket name. If WAYLAND_DISPLAY is not set, this function<br>
> + * fails and returns -1.<br>
> *<br>
> * The Unix socket will be created in the directory pointed to by environment<br>
> * variable XDG_RUNTIME_DIR. If XDG_RUNTIME_DIR is not set, then this function<br>
> @@ -1235,7 +1235,7 @@ wl_display_add_socket(struct wl_display *display, const char *name)<br>
> if (name == NULL)<br>
> name = getenv("WAYLAND_DISPLAY");<br>
> if (name == NULL)<br>
> - name = "wayland-0";<br>
> + return -1;<br>
><br>
> if (wl_socket_init_for_display_name(s, name) < 0) {<br>
> wl_socket_destroy(s);<br>
<br>
</span>wl_display_add_socket() is used by the server to create a socket. If<br>
the program does not give a name parameter, it's probably ok to try and<br>
default to "wayland-0". I am not sure why WAYLAND_DISPLAY is involved<br>
at all.<br>
<br>
Anyway, the essence of this patch is to fix libwayland-client, so this<br>
change to libwayland-server is off-topic anyway and should be<br>
considered separately.<br>
<br>
I dropped the hunk to wayland-server.c and committed the rest.<br>
<br>
Pushed:<br>
441f9bb..fb7e130 master -> master<br>
<br>
<br>
Thanks,<br>
pq<br>
</blockquote></div><br></div>