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.bet...@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 <dani...@collabora.com>
Cheers,
Daniel
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
