[Xcb] [PATCH Xlib 3/3] Add XOpenXCBConnection() to create a Display object from xcb_connection

Mike Blumenkrantz zmike at samsung.com
Thu Jun 11 11:26:13 PDT 2015


also use xcb reference counting underneath Display connection functions
to ensure matching connection object lifetimes

Commit by Josh Triplett and Jamey Sharp.

Signed-off-by: Josh Triplett <josh at joshtriplett.org>
Signed-off-by: Jamey Sharp <jamey at minilop.net>
Signed-off-by: Mike Blumenkrantz <zmike at osg.samsung.com>
---
 include/X11/Xlib-xcb.h     |   1 +
 man/Makefile.am            |   1 +
 man/XOpenDisplay.man       |  10 +++-
 man/XOpenXCBConnection.man |  40 +++++++++++++
 src/ClDisplay.c            |   8 +++
 src/OpenDis.c              | 144
++++++++++++++++++++++++++++++++-------------
src/Xxcbint.h              |   3 + 7 files changed, 163 insertions(+),
44 deletions(-) create mode 100644 man/XOpenXCBConnection.man

diff --git a/include/X11/Xlib-xcb.h b/include/X11/Xlib-xcb.h
index a21e2be..0ee8b47 100644
--- a/include/X11/Xlib-xcb.h
+++ b/include/X11/Xlib-xcb.h
@@ -11,6 +11,7 @@
 _XFUNCPROTOBEGIN
 
 xcb_connection_t *XGetXCBConnection(Display *dpy);
+Display *XOpenXCBConnection(xcb_connection_t *c);
 
 enum XEventQueueOwner { XlibOwnsEventQueue = 0, XCBOwnsEventQueue };
 void XSetEventQueueOwner(Display *dpy, enum XEventQueueOwner owner);
diff --git a/man/Makefile.am b/man/Makefile.am
index ef1a745..ad54c93 100644
--- a/man/Makefile.am
+++ b/man/Makefile.am
@@ -118,6 +118,7 @@ libman_PRE = \
 	XNextEvent.man \
 	XNoOp.man \
 	XOpenDisplay.man \
+	XOpenXCBConnection.man \
 	XOpenIM.man \
 	XOpenOM.man \
 	XParseGeometry.man \
diff --git a/man/XOpenDisplay.man b/man/XOpenDisplay.man
index 30859b2..07abd38 100644
--- a/man/XOpenDisplay.man
+++ b/man/XOpenDisplay.man
@@ -131,7 +131,8 @@ returns a pointer to a
 .ZN Display 
 structure,
 which is defined in 
-.hN X11/Xlib.h .
+.hN X11/Xlib.h . The underlying connection object is reference counted,
+and it will have a count of 1 upon returning from this call.
 If 
 .ZN XOpenDisplay 
 does not succeed, it returns NULL.
@@ -157,7 +158,8 @@ see section 2.2.1.
 .LP
 The
 .ZN XCloseDisplay
-function closes the connection to the X server for the display
specified in the +function decrements the reference count of the
underlying connection. +If the count reaches 0 it closes the connection
to the X server for the display specified in the .ZN Display
 structure and destroys all windows, resource IDs
 .Pn ( Window ,
@@ -171,7 +173,9 @@ or other resources that the client has created
 on this display, unless the close-down mode of the resource has been
changed (see
 .ZN XSetCloseDownMode ).
-Therefore, these windows, resource IDs, and other resources should
never be +Therefore, if
+.ZN XCloseDisplay
+returns 0 these windows, resource IDs, and other resources should
never be referenced again or an error will be generated.
 Before exiting, you should call
 .ZN XCloseDisplay
diff --git a/man/XOpenXCBConnection.man b/man/XOpenXCBConnection.man
new file mode 100644
index 0000000..bbb9fc5
--- /dev/null
+++ b/man/XOpenXCBConnection.man
@@ -0,0 +1,40 @@
+.\" Copyright \(co 2010 Josh Triplett, Jamey Sharp
+.\"
+.\" Permission is hereby granted, free of charge, to any person
obtaining +.\" a copy of this software and associated documentation
files (the +.\" "Software"), to deal in the Software without
restriction, including +.\" without limitation the rights to use, copy,
modify, merge, publish, +.\" distribute, sublicense, and/or sell copies
of the Software, and to +.\" permit persons to whom the Software is
furnished to do so, subject to +.\" the following conditions:
+.\"
+.\" The above copyright notice and this permission notice shall be
included +.\" in all copies or substantial portions of the Software.
+.\"
+.\" THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
EXPRESS +.\" OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+.\" MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
NONINFRINGEMENT. +.\" IN NO EVENT SHALL THE X CONSORTIUM BE LIABLE FOR
ANY CLAIM, DAMAGES OR +.\" OTHER LIABILITY, WHETHER IN AN ACTION OF
CONTRACT, TORT OR OTHERWISE, +.\" ARISING FROM, OUT OF OR IN CONNECTION
WITH THE SOFTWARE OR THE USE OR +.\" OTHER DEALINGS IN THE SOFTWARE.
+.\"
+.TH XGetXCBConnection __libmansuffix__ __xorgversion__ "XLIB FUNCTIONS"
+.SH NAME
+XOpenXCBConnection \- create an Xlib Display for an existing XCB
connection +.SH SYNTAX
+.HP
+#include <X11/Xlib-xcb.h>
+.HP
+Display *XOpenXCBConnection(xcb_connection_t *\fIc\fP);
+.SH ARGUMENTS
+.IP \fIc\fP 1i
+Specifies the xcb_connection_t to open a Display for.
+.SH DESCRIPTION
+The \fIXOpenXCBConnection\fP function returns the Xlib Display
associated with +an XCB connection, creating that Display if none
exists. The connection object +is reference counted and will only be
deleted when all references are removed. +.SH "SEE ALSO"
+XOpenDisplay(__libmansuffix__),
+.br
+\fIXlib \- C Language X Interface\fP
diff --git a/src/ClDisplay.c b/src/ClDisplay.c
index bddd773..3ad2d89 100644
--- a/src/ClDisplay.c
+++ b/src/ClDisplay.c
@@ -48,6 +48,12 @@ XCloseDisplay (
 	register _XExtension *ext;
 	register int i;
 
+	if (dpy->xcb->refcount > 1) {
+  _XLockMutex(_Xglobal_lock);
+		dpy->xcb->refcount--;
+  _XUnlockMutex(_Xglobal_lock);
+  return 1;
+	}
 	if (!(dpy->flags & XlibDisplayClosing))
 	{
 	    dpy->flags |= XlibDisplayClosing;
@@ -69,6 +75,8 @@ XCloseDisplay (
 		XSync(dpy, 1);
 	}
 	xcb_disconnect(dpy->xcb->connection);
+	_XLockMutex(_Xglobal_lock);
 	_XFreeDisplayStructure (dpy);
+	_XUnlockMutex(_Xglobal_lock);
 	return 0;
 }
diff --git a/src/OpenDis.c b/src/OpenDis.c
index 509060e..d62cf14 100644
--- a/src/OpenDis.c
+++ b/src/OpenDis.c
@@ -54,6 +54,8 @@ static xReq _dummy_request = {
 	0, 0, 0
 };
 
+static Display *dpy_head;
+
 static void OutOfMemory(Display *dpy);
 
 /*
@@ -66,11 +68,59 @@ XOpenDisplay (
 {
 	xcb_connection_t *c;
 	register Display *dpy;		/* New Display object
being created. */
+	char *display_name;		/* pointer to display name
*/
+	int iscreen;			/* screen number */
+	/*
+	 * If the display specifier string supplied as an argument to
this
+	 * routine is NULL or a pointer to NULL, read the DISPLAY
variable.
+	 */
+	if (display == NULL || *display == '\0') {
+		if ((display_name = getenv("DISPLAY")) == NULL) {
+			/* Oops! No DISPLAY environment variable -
error. */
+			return(NULL);
+		}
+	}
+	else {
+		/* Display is non-NULL, copy the pointer */
+		display_name = (char *)display;
+	}
+
+/*
+ * Call the Connect routine to get the transport connection object.
+ * If NULL is returned, the connection failed.
+ */
+	if(!(c = _XConnectXCB(display, &iscreen))) {
+		return NULL;
+	}
+
+	dpy = XOpenXCBConnection(c);
+         /* Drop the reference taken by XOpenXCBConnection; Xlib owns
this
+          * connection. */
+        xcb_disconnect(c);
+
+	if ((dpy->display_name = strdup(display_name)) == NULL) {
+		OutOfMemory(dpy);
+		return(NULL);
+	}
+
+/*
+ * Make sure default screen is legal.
+ */
+	dpy->default_screen = iscreen;  /* Value returned by
ConnectDisplay */
+	if (iscreen >= dpy->nscreens) {
+	    OutOfMemory(dpy);
+	    return(NULL);
+	}
+
+	return dpy;
+}
+
+static Display *_XOpenXCBConnection(xcb_connection_t *c)
+{
+	Display *dpy;			/* New Display object
being created. */ register int i;
 	int j, k;			/* random iterator indexes */
-	char *display_name;		/* pointer to display name
*/ char *setup = NULL;		/* memory allocated at startup */
-	int iscreen;			/* screen number */
 	xConnSetupPrefix prefix;	/* prefix information */
 	int vendorlen;			/* length of vendor
string */ union {
@@ -86,22 +136,13 @@ XOpenDisplay (
 	long usedbytes = 0;     /* number of bytes we have processed */
 	unsigned long mask;
 	long int conn_buf_size;
-	char *xlib_buffer_size;
+	char *xlib_buffer_size, *display_name;
 
-	/*
-	 * If the display specifier string supplied as an argument to
this
-	 * routine is NULL or a pointer to NULL, read the DISPLAY
variable.
-	 */
-	if (display == NULL || *display == '\0') {
-		if ((display_name = getenv("DISPLAY")) == NULL) {
-			/* Oops! No DISPLAY environment variable -
error. */
-			return(NULL);
-		}
-	}
-	else {
-		/* Display is non-NULL, copy the pointer */
-		display_name = (char *)display;
-	}
+	for (dpy = dpy_head; dpy; dpy = dpy->xcb->dpy_next)
+		if (dpy->xcb->connection == c)
+			return dpy;
+
+        xcb_reference(c);
 
 /*
  * Set the default error handlers.  This allows the global variables to
@@ -111,15 +152,6 @@ XOpenDisplay (
 	if (_XIOErrorFunction == NULL) (void) XSetIOErrorHandler
(NULL); 
 /*
- * Call the Connect routine to get the transport connection object.
- * If NULL is returned, the connection failed.
- */
-
-	if(!(c = _XConnectXCB(display, &iscreen))) {
-		return NULL;
-	}
-
-/*
  * Attempt to allocate a display structure. Return NULL if allocation
fails. */
 	if ((dpy = (Display *)Xcalloc(1, sizeof(Display) +
sizeof(_X11XCBPrivate))) == NULL) { @@ -130,12 +162,6 @@ XOpenDisplay (
 	dpy->fd = xcb_get_file_descriptor(c);
 	dpy->xcb = (_X11XCBPrivate *) (dpy + 1);
 	dpy->xcb->connection = c;
-
-	if ((dpy->display_name = strdup(display_name)) == NULL) {
-		OutOfMemory(dpy);
-		return NULL;
-	}
-
 	dpy->xcb->next_xid = xcb_generate_id(dpy->xcb->connection);
 
 	dpy->xcb->event_notify = xcondition_malloc();
@@ -147,6 +173,19 @@ XOpenDisplay (
 	xcondition_init(dpy->xcb->event_notify);
 	xcondition_init(dpy->xcb->reply_notify);
 
+	/* Not necessarily correct, but this can only go wrong if
opening an
+	 * xcb_connection_t using a display_name other than $DISPLAY,
*and*
+	 * using XOpenXCBConnection rather than XOpenDisplay, *and*
then
+	 * attempting to reconnect to dpy->display_name. */
+	display_name = getenv("DISPLAY");
+	if (display_name != NULL) {
+		dpy->display_name = strdup(display_name);
+		if (!dpy->display_name) {
+			OutOfMemory(dpy);
+			return NULL;
+		}
+	}
+
 	/* Initialize as much of the display structure as we can.
 	 * Initialize pointers to NULL so that XFreeDisplayStructure
will
 	 * work if we run out of memory before we finish initializing.
@@ -215,7 +254,6 @@ XOpenDisplay (
 	dpy->savedsynchandler = NULL;
 	dpy->request = 0;
 	dpy->last_request_read = 0;
-	dpy->default_screen = iscreen;  /* Value returned by
ConnectDisplay */ dpy->last_req = (char *)&_dummy_request;
 
 	/* Initialize the display lock */
@@ -501,14 +539,6 @@ XOpenDisplay (
  */
 
 /*
- * Make sure default screen is legal.
- */
-	if (iscreen >= dpy->nscreens) {
-	    OutOfMemory(dpy);
-	    return(NULL);
-	}
-
-/*
  * get availability of large requests
  */
 	dpy->bigreq_size =
xcb_get_maximum_request_length(dpy->xcb->connection); @@ -575,12
+605,37 @@ XOpenDisplay ( #ifdef XKB
 	XkbUseExtension(dpy,NULL,NULL);
 #endif
+
+	dpy->xcb->dpy_next = dpy_head;
+	dpy_head = dpy;
+
 /*
  * and return successfully
  */
  	return(dpy);
 }
 
+/*
+ * Construct a Display from an existing xcb_connection_t.
+ */
+Display *XOpenXCBConnection(xcb_connection_t *c)
+{
+	Display *dpy;
+/*
+ * Set the default error handlers here to prevent deadlocks later.
+ * This allows the global variables to default to NULL for use with
shared libraries.
+ */
+	if (_XErrorFunction == NULL) (void) XSetErrorHandler (NULL);
+	if (_XIOErrorFunction == NULL) (void) XSetIOErrorHandler
(NULL);
+	_XLockMutex(_Xglobal_lock);
+	dpy = _XOpenXCBConnection(c);
+	if (dpy)
+		dpy->xcb->refcount++;
+	_XUnlockMutex(_Xglobal_lock);
+	return dpy;
+}
+
+
 /* XFreeDisplayStructure frees all the storage associated with a
  * Display.  It is used by XOpenDisplay if it runs out of memory,
  * and also by XCloseDisplay.   It needs to check whether all pointers
@@ -592,6 +647,13 @@ XOpenDisplay (
 
 void _XFreeDisplayStructure(Display *dpy)
 {
+	Display **dpy_cur;
+	for (dpy_cur = &dpy_head; *dpy_cur; dpy_cur =
&(*dpy_cur)->xcb->dpy_next)
+		if (*dpy_cur == dpy) {
+			*dpy_cur = dpy->xcb->dpy_next;
+			break;
+		}
+
 	/* move all cookies in the EQ to the jar, then free them. */
 	if (dpy->qfree) {
 	    _XQEvent *qelt = dpy->qfree;
diff --git a/src/Xxcbint.h b/src/Xxcbint.h
index b4f8988..f0a9e6a 100644
--- a/src/Xxcbint.h
+++ b/src/Xxcbint.h
@@ -39,6 +39,9 @@ typedef struct _X11XCBPrivate {
 	xcondition_t event_notify;
 	int event_waiter;
 	xcondition_t reply_notify;
+
+	int refcount;
+	Display *dpy_next;
 } _X11XCBPrivate;
 
 /* xcb_disp.c */
-- 
2.4.2



More information about the Xcb mailing list