[Xcb] [PATCH 2/4] xcb_connect*: make cancellation point instead of disabling cancellation

Rémi Denis-Courmont remi at remlab.net
Thu Jan 7 09:25:13 PST 2010


---
 src/xcb.h      |    7 +++++++
 src/xcb_auth.c |    6 ++----
 src/xcb_conn.c |   35 ++++++++++++++++++++---------------
 src/xcb_util.c |   17 ++++++++++++++---
 4 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/src/xcb.h b/src/xcb.h
index f951276..ced755b 100644
--- a/src/xcb.h
+++ b/src/xcb.h
@@ -383,6 +383,9 @@ int xcb_connection_has_error(xcb_connection_t *c);
  * bidirectionally connected to an X server. If the connection
  * should be unauthenticated, @p auth_info must be @c
  * NULL.
+ *
+ * This is a thread cancellation point. In case of cancellation, the
+ * file descriptor is closed.
  */
 xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info);
 
@@ -428,6 +431,8 @@ int xcb_parse_display(const char *name, char **host, int *display, int *screen);
  * variable. If a particular screen on that server is preferred, the
  * int pointed to by @p screenp (if not @c NULL) will be set to that
  * screen; otherwise the screen will be set to 0.
+ *
+ * This is a thread cancellation point.
  */
 xcb_connection_t *xcb_connect(const char *displayname, int *screenp);
 
@@ -442,6 +447,8 @@ xcb_connection_t *xcb_connect(const char *displayname, int *screenp);
  * authorization @p auth. If a particular screen on that server is
  * preferred, the int pointed to by @p screenp (if not @c NULL) will
  * be set to that screen; otherwise @p screenp will be set to 0.
+ *
+ * This is a thread cancellation point.
  */
 xcb_connection_t *xcb_connect_to_display_with_auth_info(const char *display, xcb_auth_info_t *auth, int *screen);
 
diff --git a/src/xcb_auth.c b/src/xcb_auth.c
index f98dee1..d77b131 100644
--- a/src/xcb_auth.c
+++ b/src/xcb_auth.c
@@ -136,16 +136,14 @@ static Xauth *get_authptr(struct sockaddr *sockname, unsigned int socknamelen,
     /* snprintf may have truncate our text */
     dispbuflen = MIN(dispbuflen, sizeof(dispbuf) - 1);
 
-    pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &canc);
     if (family == FamilyLocal) {
-        if (gethostname(hostnamebuf, sizeof(hostnamebuf)) == -1) {
-            pthread_setcancelstate(canc, NULL);
+        if (gethostname(hostnamebuf, sizeof(hostnamebuf)) == -1)
             return 0;   /* do not know own hostname */
-        }
         addr = hostnamebuf;
         addrlen = strlen(addr);
     }
 
+    pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &canc);
     xauth = XauGetBestAuthByAddr (family,
                                   (unsigned short) addrlen, addr,
                                   (unsigned short) dispbuflen, dispbuf,
diff --git a/src/xcb_conn.c b/src/xcb_conn.c
index 23b2eb1..5786f28 100644
--- a/src/xcb_conn.c
+++ b/src/xcb_conn.c
@@ -52,18 +52,15 @@ static const int error_connection = 1;
 
 static int set_fd_flags(const int fd)
 {
-    int flags;
-    int cancel;
-    int ret = 0;
-
-    pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cancel);
-    flags = fcntl(fd, F_GETFL, 0);
-    if(flags != -1 &&
-       fcntl(fd, F_SETFL, flags | O_NONBLOCK) != -1 &&
-       fcntl(fd, F_SETFD, FD_CLOEXEC) != -1)
-        ret = 1;
-    pthread_setcancelstate(cancel, NULL);
-    return ret;
+    int flags = fcntl(fd, F_GETFL, 0);
+    if(flags == -1)
+        return 0;
+    flags |= O_NONBLOCK;
+    if(fcntl(fd, F_SETFL, flags) == -1)
+        return 0;
+    if(fcntl(fd, F_SETFD, FD_CLOEXEC) == -1)
+        return 0;
+    return 1;
 }
 
 static int write_setup(xcb_connection_t *c, xcb_auth_info_t *auth_info)
@@ -194,6 +191,11 @@ static int write_vec(xcb_connection_t *c, struct iovec **vector, int *count)
     return 1;
 }
 
+static void _xcb_cleanup_fd(void *data)
+{
+    close((intptr_t)data);
+}
+
 /* Public interface */
 
 const xcb_setup_t *xcb_get_setup(xcb_connection_t *c)
@@ -221,6 +223,7 @@ int xcb_connection_has_error(xcb_connection_t *c)
 xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info)
 {
     xcb_connection_t* c;
+    int val;
 
     c = calloc(1, sizeof(xcb_connection_t));
     if(!c) {
@@ -230,7 +233,8 @@ xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info)
 
     c->fd = fd;
 
-    if(!(
+    pthread_cleanup_push(_xcb_cleanup_fd, (void *)(intptr_t)fd);
+    val =
         set_fd_flags(fd) &&
         pthread_mutex_init(&c->iolock, 0) == 0 &&
         _xcb_in_init(&c->in) &&
@@ -238,8 +242,9 @@ xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info)
         write_setup(c, auth_info) &&
         read_setup(c) &&
         _xcb_ext_init(c) &&
-        _xcb_xid_init(c)
-        ))
+        _xcb_xid_init(c);
+    pthread_cleanup_pop(0);
+    if(!val)
     {
         xcb_disconnect(c);
         return (xcb_connection_t *) &error_connection;
diff --git a/src/xcb_util.c b/src/xcb_util.c
index 325681a..3b6fcea 100644
--- a/src/xcb_util.c
+++ b/src/xcb_util.c
@@ -120,6 +120,11 @@ int xcb_parse_display(const char *name, char **host, int *displayp,
     return _xcb_parse_display(name, host, NULL, displayp, screenp);
 }
 
+static void cleanup_file(void *data)
+{
+    close((intptr_t)data);
+}
+
 static int _xcb_open_tcp(char *host, char *protocol, const unsigned short port);
 static int _xcb_open_unix(char *protocol, const char *file);
 #ifdef DNETCONN
@@ -358,6 +363,7 @@ xcb_connection_t *xcb_connect_to_display_with_auth_info(const char *displayname,
     char *protocol;
     xcb_auth_info_t ourauth;
     xcb_connection_t *c;
+    int val;
 
     int parsed = _xcb_parse_display(displayname, &host, &protocol, &display, screenp);
     
@@ -380,11 +386,16 @@ xcb_connection_t *xcb_connect_to_display_with_auth_info(const char *displayname,
     if(auth)
         return xcb_connect_to_fd(fd, auth);
 
-    if(_xcb_get_auth_info(fd, &ourauth, display))
+    pthread_cleanup_push(cleanup_file, (void *)(intptr_t)fd);
+    val = _xcb_get_auth_info(fd, &ourauth, display);
+    pthread_cleanup_pop(0);
+    if(val)
     {
+        pthread_cleanup_push(free, ourauth.name);
+        pthread_cleanup_push(free, ourauth.data);
         c = xcb_connect_to_fd(fd, &ourauth);
-        free(ourauth.name);
-        free(ourauth.data);
+        pthread_cleanup_pop(1);
+        pthread_cleanup_pop(1);
     }
     else
         c = xcb_connect_to_fd(fd, 0);
-- 
1.6.6



More information about the Xcb mailing list