[Xcb] [Bug 69118] Unix spool is located at /var/spool/sockets/X11/... or /usr/spool/sockets/X11/... under HPUX

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Sat Sep 14 13:35:05 PDT 2013


https://bugs.freedesktop.org/show_bug.cgi?id=69118

--- Comment #6 from Uli Schlachter <psychon at znc.in> ---
Comment on attachment 85472
  --> https://bugs.freedesktop.org/attachment.cgi?id=85472
Clean up code for picking path for UNIX socket files

Review of attachment 85472:
-----------------------------------------------------------------

Ok, I give up. According to the commit message, I would expect no behavior
changes from this patch, but this code and the patch are complicated enough
that I am not convinced about this.

Any ideas on how this could be made easier to review? Perhaps splitting up into
more individual patches? (Sorry!)

::: src/xcb_util.c
@@ +81,5 @@
> +static int _xcb_add_socket_path(const char** socket_paths, int max_cnt, int* cnt,
> +				const char* socket_path)
> +{
> +    if (*cnt >= max_cnt)
> +	return -1;

I know I will eventually regret this, but I would prefer this to be an assert()
(mostly because the return value of this function is completely ignored).

@@ +255,5 @@
>      file = malloc(filelen);
>      if(file == NULL)
>          return -1;
>  
> +    for(i = 0; i < socket_path_cnt; ++i) {

This is another behavior change compared to the old code, isn't it? The old
code only tried a single place and then gave up (at least sometimes, this is
all hard to follow).

@@ +260,3 @@
>  #ifdef HAVE_LAUNCHD
> +	if(strncmp(socket_paths[i], "/tmp/launch", 11) == 0)
> +	    actual_filelen = snprintf(file, filelen, "%s%c%d", socket_paths[i], ':', display);

Pfft, why did you move the ':' out via a new %c argument? This is just a
constant character...

@@ -240,4 @@
>  
> -    if (fd < 0 && !protocol && *host == '\0') {
> -	    unsigned short port = X_TCP_PORT + display;
> -	    fd = _xcb_open_tcp(host, protocol, port);

This code is lost in the new version, isn't it?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/xcb/attachments/20130914/7fb8df42/attachment.html>


More information about the Xcb mailing list