[PATCH wayland] extend WAYLAND_DISPLAY semantic and make server socket path configurable using environment

[Daniel Stone]

Hi Davide,
This commit is actually several in one: can you please separate them?
When you do, please don't indent the commit message either.

On 24 February 2015 at 14:08, Davide Bettio <davide.bettio at ispirata.com> wrote:
>     * Extend WAYLAND_DISPLAY and name parameter semantics to support
> absolute paths.
>     For example WAYLAND_DISPLAY="/my/path/wayland-2" or
> connect_to_socket("/my/path/wayland-2").

This would be commit #1.

>     * Introduce WAYLAND_SERVER_SOCKET_DIR to change socket directory without
> changing XDG_RUNTIME_DIR.
>     This might be useful to change socket directory in case
> wl_display_add_socket_auto is used.
>     For example this will change the socket path for weston without using
> any command line argument.
>     This envvar is ignored when WAYLAND_SERVER_SOCKET is set to an absolute
> path.

Commit #2.

>     * Introduce WAYLAND_SERVER_SOCKET to change the path where the server
> will create the socket.
>     It will be possible for a nested compositor to offer a socket located in
> WAYLAND_SERVER_SOCKET path
>     while connecting to the main compositor socket that is located in
> WAYLAND_DISPLAY path.
>     That's the reason why a futher WAYLAND_SERVER_SOCKET envvar has been
> introduced.

Commit #3.

>     Wayland clients will not see any new environment variable, while
> compositors may have to deal
>     with 2 extra environment variables, this is due the fact that a
> compositor might be a client of
>     another compositor and we must make sure that WAYLAND_DISPLAY is not
> used for both client
>     and server purposes.

Isn't this already the case today?
>         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 (name[0] != '/') {
> +                       wl_log("error: socket path \"%s/%s\" plus null
> terminator"
> +                              " exceeds 108 bytes\n", runtime_dir, name);
> +               } else {
> +                       wl_log("error: socket path \"%s\" plus null
> terminator"
> +                              " exceeds 108 bytes\n", name);
> +               }

I know this isn't actually your doing, but changing the hardcoded 108
to sizeof(addr.sun_path) at the same time would be nice.

> +       } else {
> +               //absolute path

C-style comments (/* */) please.

> + * If NULL is passed as name, then it would look in order for
> WAYLAND_SERVER_SOCKET
> + * and WAYLAND_SERVER

s/WAYLAND_SERVER/WAYLAND_DISPLAY/

> + * The Unix socket will be created in the directory pointed to by either
> environment
> + * variable WAYLAND_SERVER_SOCKET_DIR or XDG_RUNTIME_DIR. If the none of
> the is set,
> + * then this function  fails and returns -1.

s/the none/neither/ (sorry, extreme nitpicking now)

>   *
> - * The length of socket path, i.e., the path set in XDG_RUNTIME_DIR and the
> + * The length of socket path, e.g., the path set in XDG_RUNTIME_DIR and the

i.e. is correct - i.e. means 'that is' or 'specifically', whereas e.g.
means 'for example'.

With these comments addressed and the commit split:
Reviewed-by: Daniel Stone <daniels at collabora.com>

Cheers,
Daniel