[PATCH libX11-xcb] add XCBGetXDisplay

[Mike Blumenkrantz]

On Wed, 27 May 2015 20:57:43 -0400
Mike Blumenkrantz <zmike at samsung.com> wrote:

> 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

Hi,

Are there any further comments, or can this be pushed upstream?

Thanks,
Mike