[PATCH libX11-xcb] add XCBGetXDisplay

[Mike Blumenkrantz]

On Wed, 27 May 2015 22:18:08 +0200
Uli Schlachter <psychon at znc.in> wrote:

> Hi,
> 
> Am 27.05.2015 um 14:42 schrieb Mike Blumenkrantz:
> [...]
> > From c5e4bf50a722670e0ac1b05509834874251c0c9c Mon Sep 17 00:00:00
> > 2001 From: Mike Blumenkrantz <zmike at osg.samsung.com>
> > Date: Wed, 27 May 2015 08:37:05 -0400
> > Subject: [PATCH] add XCBGetXDisplay
> > 
> > a method for creating a Display object from an xcb connection
> 
> This commit message might be improved a bit.

I've added more details.

> 
> > Signed-off-by: Mike Blumenkrantz <zmike at osg.samsung.com>
> > ---
> >  include/X11/Xlib-xcb.h |   1 +
> >  src/x11_xcb.c          | 562
> > +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed,
> > 563 insertions(+)
> > 
> > diff --git a/include/X11/Xlib-xcb.h b/include/X11/Xlib-xcb.h
> > index a21e2be..240a25c 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 *XCBGetXDisplay(xcb_connection_t *c);
> >  
> >  enum XEventQueueOwner { XlibOwnsEventQueue = 0,
> > XCBOwnsEventQueue }; void XSetEventQueueOwner(Display *dpy, enum
> > XEventQueueOwner owner); diff --git a/src/x11_xcb.c b/src/x11_xcb.c
> > index 3ddf403..0dabf6d 100644
> > --- a/src/x11_xcb.c
> > +++ b/src/x11_xcb.c
> > @@ -1,9 +1,78 @@
> [...]
> 
> Where did you copy these more than 500 lines from and why can't it be
> shared with XOpenDisplay() (I guess that is where this code comes
> from)?

It was recommended to me by Daniel Stone that I not touch anything
directly in libX11 to avoid potentially introducing regressions, so I
did copy most of XOpenDisplay() here.

> 
> > +	if ((display_name = getenv("DISPLAY")) == NULL) {
> > +		/* Oops! No DISPLAY environment variable - error.
> > */
> > +		return(NULL);
> > +	}
> 
> Why does this function require $DISPLAY? That is totally unrelated to
> the XCB connection that you give it.

True, I'll fix that.

> 
> Also, there is no documentation for this new function and I guess
> calling XCloseDisplay() on it will also close the XCB connection.
> That doesn't seem right.

Thanks for the pointer about documentation, I forgot to add it.

It's my assumption that anyone using this function will be using it to
manage their connection with libX11, and so this is the desired
behavior. It will be specified in the documentation.

> 
> Cheers,
> Uli

Thanks for these suggestions. I have made some updates and attached a
new version of the patch.

Regards,
Mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-add-XCBGetXDisplay-to-x11-xcb.patch
Type: text/x-patch
Size: 19357 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20150527/f91ea1c1/attachment.bin>