[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 xorg-devel
mailing list