On Mon, 27 Nov 2017 08:54:54 -0600 Matt Hoosier <matt.hoos...@gmail.com> wrote:
> From: Matt Hoosier <matt.hoos...@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. > > v4 changes: > > * Improved internal comments and some boundary-condition > error checks in test case. > * Refer to compositor as "Wayland server" rather than "Wayland > display" in wl_display_connect() doxygen comments. > * Remove redundant descriptions of parameter-interpretation > mechanics from wl_display_connect() manpage. Reworked things > to make it clear that 'name' and $WAYLAND_DISLAY are each > capable of encoding absolute server socket paths. > * Remove callout to reference implementation behavior in protocol > documented. In its place there is now a simple statement that > implementations can optionally support absolute socket paths. > > v3 changes: > > * Added test case. > * Clarified documentation to note that 'name' parameter to > wl_display_connect() > can also be an absolute path. > > 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.hoos...@gmail.com> > Acked-by: Pekka Paalanen <pekka.paala...@collabora.co.uk> > Acked-by: Jonas Ådahl <jad...@gmail.com> > --- > doc/man/wl_display_connect.xml | 32 +++++++++-- > doc/publican/sources/Protocol.xml | 5 +- > src/wayland-client.c | 47 ++++++++++++---- > tests/socket-test.c | 109 > ++++++++++++++++++++++++++++++++++++++ > 4 files changed, 177 insertions(+), 16 deletions(-) Hi Matt, this patch is: Reviewed-by: Pekka Paalanen <pekka.paala...@collabora.co.uk> The wording in the man page sounds little like WAYLAND_DISPLAY accepting an absolute path is a side-effect rather than an intentional feature, but it doesn't matter. Everyone, if there are no objections, I will push this patch on Wednesday, that is in two days. If you want your R-b or Acks recorded that are not already in the above, please send them explicitly. Thanks, pq > > diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml > index 7e6e05c..dab4ddb 100644 > --- a/doc/man/wl_display_connect.xml > +++ b/doc/man/wl_display_connect.xml > @@ -55,15 +55,39 @@ > <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> when > <envar>WAYLAND_DISPLAY</envar> > + (or <varname>name</varname>, see below) is a simple name, for this > + function to find it. The server socket is also allowed to exist at an > + arbitrary path; usage details follow. See below for compatibility > issue > + details.</para> > + > + <para>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 > + <envar>WAYLAND_DISPLAY</envar> replaces the default value. > + > + If <varname>name</varname> is an absolute path, then that path is used > + as the Wayland socket to which the connection is attempted. Note that > + in combination with the default-value behavior described above, this > + implies that setting <envar>WAYLAND_DISPLAY</envar> to an absolute > + path will implicitly cause <varname>name</varname> to take on that > + absolute path if <varname>name</varname> is <constant>NULL</constant>. > + > + 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> > > + <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> > + > <para><function>wl_display_connect_to_fd</function> connects to a Wayland > socket with an explicit file-descriptor. The file-descriptor is > passed > as argument <varname>fd</varname>.</para> > diff --git a/doc/publican/sources/Protocol.xml > b/doc/publican/sources/Protocol.xml > index ba6b5f1..9fdee9a 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). Beginning in Wayland 1.15, implementations can > + optionally support server socket endpoints located at arbitrary > + locations in the filesystem by setting > <emphasis>WAYLAND_DISPLAY</emphasis> > + to the absolute path at which the server endpoint listens. > </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..ecb8840 100644 > --- a/src/wayland-client.c > +++ b/src/wayland-client.c > @@ -857,9 +857,17 @@ connect_to_socket(const char *name) > socklen_t size; > const char *runtime_dir; > int name_size, fd; > + bool path_is_absolute; > + > + if (name == NULL) > + name = getenv("WAYLAND_DISPLAY"); > + if (name == NULL) > + name = "wayland-0"; > + > + path_is_absolute = name[0] == '/'; > > runtime_dir = getenv("XDG_RUNTIME_DIR"); > - if (!runtime_dir) { > + if (!runtime_dir && !path_is_absolute) { > wl_log("error: XDG_RUNTIME_DIR not set in the environment.\n"); > /* to prevent programs reporting > * "failed to create display: Success" */ > @@ -867,25 +875,32 @@ connect_to_socket(const char *name) > return -1; > } > > - if (name == NULL) > - name = getenv("WAYLAND_DISPLAY"); > - if (name == NULL) > - name = "wayland-0"; > - > fd = wl_os_socket_cloexec(PF_LOCAL, SOCK_STREAM, 0); > if (fd < 0) > return -1; > > memset(&addr, 0, sizeof addr); > addr.sun_family = AF_LOCAL; > - name_size = > - snprintf(addr.sun_path, sizeof addr.sun_path, > - "%s/%s", runtime_dir, name) + 1; > + if (!path_is_absolute) { > + name_size = > + snprintf(addr.sun_path, sizeof addr.sun_path, > + "%s/%s", runtime_dir, name) + 1; > + } else { > + /* absolute path */ > + name_size = > + snprintf(addr.sun_path, sizeof addr.sun_path, > + "%s", name) + 1; > + } > > assert(name_size > 0); > if (name_size > (int)sizeof addr.sun_path) { > - wl_log("error: socket path \"%s/%s\" plus null terminator" > - " exceeds 108 bytes\n", runtime_dir, name); > + if (!path_is_absolute) { > + wl_log("error: socket path \"%s/%s\" plus null > terminator" > + " exceeds %i bytes\n", runtime_dir, name, (int) > sizeof(addr.sun_path)); > + } else { > + wl_log("error: socket path \"%s\" plus null terminator" > + " exceeds %i bytes\n", name, (int) > sizeof(addr.sun_path)); > + } > close(fd); > /* to prevent programs reporting > * "failed to add socket: Success" */ > @@ -994,6 +1009,16 @@ 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 an absolute path, then that path is used as-is for > + * the location of the socket at which the Wayland server is listening; > + * no qualification inside XDG_RUNTIME_DIR is attempted. > + * > + * If \c name is \c NULL and the WAYLAND_DISPLAY environment variable > + * is set to an absolute pathname, then that pathname is used as-is > + * for the socket in the same manner as if \c name held an absolute > + * path. Support for absolute paths in \c name and WAYLAND_DISPLAY > + * is present since Wayland version 1.15. > + * > * \memberof wl_display > */ > WL_EXPORT struct wl_display * > diff --git a/tests/socket-test.c b/tests/socket-test.c > index bb034f4..e970562 100644 > --- a/tests/socket-test.c > +++ b/tests/socket-test.c > @@ -26,10 +26,14 @@ > #include <errno.h> > #include <string.h> > #include <stdio.h> > +#include <sys/socket.h> > +#include <sys/types.h> > +#include <sys/wait.h> > #include <sys/un.h> > #include <unistd.h> > > #include "wayland-client.h" > +#include "wayland-os.h" > #include "wayland-server.h" > #include "test-runner.h" > > @@ -173,3 +177,108 @@ TEST(add_socket_auto) > > wl_display_destroy(d); > } > + > +struct client_create_listener { > + struct wl_listener listener; > + struct wl_display *display; > +}; > + > +struct client_destroy_listener { > + struct wl_listener listener; > + struct wl_display *display; > +}; > + > +static void > +client_destroy_notify(struct wl_listener *l, void *data) > +{ > + struct client_destroy_listener *listener = > + wl_container_of(l, listener, listener); > + wl_display_terminate(listener->display); > + free(listener); > +} > + > +static void > +client_create_notify(struct wl_listener *l, void *data) > +{ > + struct wl_client *client = data; > + struct client_create_listener *listener = > + wl_container_of(l, listener, listener); > + struct client_destroy_listener *destroy_listener = (struct > client_destroy_listener *)malloc(sizeof *destroy_listener); > + assert(destroy_listener != NULL); > + destroy_listener->display = listener->display; > + destroy_listener->listener.notify = client_destroy_notify; > + wl_client_add_destroy_listener(client, &destroy_listener->listener); > +} > + > +TEST(absolute_socket_path) > +{ > + struct wl_display *display; > + struct client_create_listener client_create_listener; > + struct sockaddr_un addr; > + int fd; > + socklen_t size; > + const char *xdg_runtime_dir; > + size_t len; > + int ret; > + pid_t pid; > + > + /* It's a little weird that this test about absolute socket paths > + * uses XDG_RUNTIME_DIR, but that's the only location guaranteed > + * by test-runner to be both writable and unique. This isn't > + * really a problem; we'll just take care that the leaf-level > + * filename used for the socket isn't anything that would > + * accidentally be generated by a default usage of > wl_display_connect(). */ > + xdg_runtime_dir = require_xdg_runtime_dir(); > + memset(&addr, 0, sizeof addr); > + len = snprintf(addr.sun_path, sizeof addr.sun_path, > + "%s/%s", xdg_runtime_dir, "wayland-absolute-0"); > + assert(len < sizeof addr.sun_path > + && "Bug in test. Path too long"); > + > + /* The path must not exist prior to binding. */ > + assert(access(addr.sun_path, F_OK) == -1); > + > + size = offsetof (struct sockaddr_un, sun_path) + strlen(addr.sun_path); > + addr.sun_family = AF_LOCAL; > + fd = wl_os_socket_cloexec(PF_LOCAL, SOCK_STREAM, 0); > + assert(fd >= 0 ); > + ret = bind(fd, (struct sockaddr *) &addr, size); > + assert(ret >= 0); > + ret = listen(fd, 128); > + assert(ret >= 0); > + > + /* Start server display. Be careful (by avoiding > wl_display_add_socket_auto() > + * to offer only the absolutely qualified socket made above. */ > + display = wl_display_create(); > + assert(display != NULL); > + client_create_listener.listener.notify = client_create_notify; > + client_create_listener.display = display; > + wl_display_add_client_created_listener(display, > &client_create_listener.listener); > + ret = wl_display_add_socket_fd(display, fd); > + assert(ret == 0); > + > + /* Execute client that connects to the absolutely qualified server > socket path. */ > + pid = fork(); > + assert(pid != -1); > + > + if (pid == 0) { > + ret = setenv("WAYLAND_DISPLAY", addr.sun_path, 1); > + assert(ret == 0); > + struct wl_display *client_display = wl_display_connect(NULL); > + assert(client_display != NULL); > + ret = wl_display_roundtrip(client_display); > + assert(ret != -1); > + wl_display_disconnect(client_display); > + exit(0); > + assert(false); > + } > + > + wl_display_run(display); > + ret = waitpid(pid, NULL, 0); > + assert(ret == pid); > + > + wl_display_destroy(display); > + > + ret = unlink(addr.sun_path); > + assert(ret == 0); > +}
pgp38BXyMfFfR.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
