[Xcb-commit] libxcb: 2 commits - src

[Uli Schlachter]

 src/xcb.h      |    2 +-
 src/xcb_conn.c |   11 ++++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

New commits:
commit 2fb14e5883f2ea2f01d248674cfcc26ccb704753
Author: Uli Schlachter <psychon at znc.in>
Date:   Tue Dec 31 15:18:01 2013 +0100

    Make xcb_disconnect(NULL) safe
    
    Code can be simplified if the deallocation functions can always be called in
    cleanup code. So if you have some code that does several things that can go
    wrong, one of which is xcb_connect(), after this change, the xcb_connection_t*
    variable can be initialized to NULL and xcb_disconnect() can always be called on
    the connection object.
    
    References: http://lists.freedesktop.org/archives/xcb/2013-September/008659.html
    
    Signed-off-by: Uli Schlachter <psychon at znc.in>
    Reviewed-by: Julien Cristau <jcristau at debian.org>

diff --git a/src/xcb.h b/src/xcb.h
index 148b00c..c17a2ef 100644
--- a/src/xcb.h
+++ b/src/xcb.h
@@ -483,7 +483,7 @@ xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info);
  * @param c: The connection.
  *
  * Closes the file descriptor and frees all memory associated with the
- * connection @c c.
+ * connection @c c. If @p c is @c NULL, nothing is done.
  */
 void xcb_disconnect(xcb_connection_t *c);
 
diff --git a/src/xcb_conn.c b/src/xcb_conn.c
index ab901a9..fa50985 100644
--- a/src/xcb_conn.c
+++ b/src/xcb_conn.c
@@ -350,7 +350,7 @@ xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info)
 
 void xcb_disconnect(xcb_connection_t *c)
 {
-    if(is_static_error_conn(c))
+    if(c == NULL || is_static_error_conn(c))
         return;
 
     free(c->setup);
commit 4dcbfd77b78ca6b016ce815af26235501f6cd75a
Author: Uli Schlachter <psychon at znc.in>
Date:   Tue Dec 31 15:05:36 2013 +0100

    xcb_disconnect(): Fix leak with error connections
    
    There are two kind of error connections in XCB. First, if something goes wrong
    while the connection is being set up, _xcb_conn_ret_error() is used to return a
    static connection in an error state. If something goes wrong later,
    _xcb_conn_shutdown() is used to set c->has_error.
    
    This is important, because the static object that _xcb_conn_ret_error() returns
    must not be freed, while the dynamically allocated objects that go through
    _xcb_conn_shutdown() must obviously be properly deallocated.
    
    This used to work correctly, but in 769acff0da8, xcb_disconnect() was made to
    ignore all connections in an error state completely. Fix this by only ignoring
    the few static error connections that we have.
    
    This was tested with the following hack:
    
        xcb_connection_t *c = xcb_connect(NULL, NULL);
        close(xcb_get_file_descriptor(c));
        xcb_discard_reply(c, xcb_get_input_focus(c).sequence);
        xcb_flush(c);
        xcb_disconnect(c);
    
    Valgrind confirms that xcb has a memory leak before this patch that this patch
    indeed fixes.
    
    Signed-off-by: Uli Schlachter <psychon at znc.in>
    Reviewed-by: Julien Cristau <jcristau at debian.org>

diff --git a/src/xcb_conn.c b/src/xcb_conn.c
index ae51b93..ab901a9 100644
--- a/src/xcb_conn.c
+++ b/src/xcb_conn.c
@@ -64,11 +64,20 @@ typedef struct {
     uint16_t length;
 } xcb_setup_generic_t;
 
+/* Keep this list in sync with is_static_error_conn()! */
 static const int xcb_con_error = XCB_CONN_ERROR;
 static const int xcb_con_closed_mem_er = XCB_CONN_CLOSED_MEM_INSUFFICIENT;
 static const int xcb_con_closed_parse_er = XCB_CONN_CLOSED_PARSE_ERR;
 static const int xcb_con_closed_screen_er = XCB_CONN_CLOSED_INVALID_SCREEN;
 
+static int is_static_error_conn(xcb_connection_t *c)
+{
+    return c == (xcb_connection_t *) &xcb_con_error ||
+           c == (xcb_connection_t *) &xcb_con_closed_mem_er ||
+           c == (xcb_connection_t *) &xcb_con_closed_parse_er ||
+           c == (xcb_connection_t *) &xcb_con_closed_screen_er;
+}
+
 static int set_fd_flags(const int fd)
 {
 /* Win32 doesn't have file descriptors and the fcntl function. This block sets the socket in non-blocking mode */
@@ -341,7 +350,7 @@ xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info)
 
 void xcb_disconnect(xcb_connection_t *c)
 {
-    if(c->has_error)
+    if(is_static_error_conn(c))
         return;
 
     free(c->setup);