[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