[PATCH] XOpenDisplay: save the correct display_name value
Aaron Plattner
aplattner at nvidia.com
Mon Aug 16 09:34:04 PDT 2010
The X Test Suite's XDisplayString test checks the invariant
XDisplayString(XOpenDisplay(str)) == str. The XCB-based XOpenDisplay violates
this invariant by expanding str to the canonical form "host:display.scrn"
(unless HAVE_LAUNCHD is set and it starts with "/tmp/launch"). E.g., this
expands ":1" to ":1.0":
400|26 1 1 19:26:41|IC Start
200|26 1 19:26:41|TP Start
520|26 1 00032625 1 1|VSW5TESTSUITE PURPOSE 1
520|26 1 00032625 1 2|Assertion XDisplayString-1.(A)
520|26 1 00032625 1 3|A call to XDisplayString returns the string that was used
520|26 1 00032625 1 4|as the argument to the XOpenDisplay call that returned the
520|26 1 00032625 1 5|value used as the display argument.
520|26 1 00032625 1 6|METH: Open a connection using XOpenDisplay.
520|26 1 00032625 1 7|METH: Obtain the display string using XDisplayString.
520|26 1 00032625 1 8|METH: Verify that the value of the string is the parameter used in XOpenDisplay.
520|26 1 00032625 1 9|METH: Close the display using XCloseDisplay.
520|26 1 00032625 1 10|REPORT: XDisplayString() returned ":1.0" instead of ":1".
220|26 1 1 19:26:41|FAIL
410|26 1 1 19:26:41|IC End
Fix this by deleting all of the code to construct the canonical path and just
stash a copy of the original display_name in dpy->display_name.
Delete the now unused HAVE_LAUNCHD macro and code to set it.
Signed-off-by: Aaron Plattner <aplattner at nvidia.com>
---
I don't really know whether this is the correct fix, but XTS clearly
expects XDisplayString to return exactly the same string that was passed
into XOpenDisplay:
if(strcmp(rdispstr, config.display) != 0) {
report("%s() returned \"%s\" instead of \"%s\".", TestName, rdispstr, config.display);
FAIL;
However, this behavior has been in Xlib for a long time and it seems like
some application might rely on it. As an alternative, I could update the
test to allow XDisplayString to tack on a screen number if the original
display string didn't have one, but the man page does say,
"The DisplayString macro returns the string that was passed to
XOpenDisplay when the current display was opened."
Jeremy, you seem to the be the most familiar with launchd. Does this
change seem okay to you? Since this change stops appending the screen
number all the time, it seems like it should be a no-op for the launchd
case.
configure.ac | 11 -----------
src/OpenDis.c | 20 ++++++++++----------
src/Xxcbint.h | 2 +-
src/xcb_disp.c | 18 ++----------------
4 files changed, 13 insertions(+), 38 deletions(-)
diff --git a/configure.ac b/configure.ac
index efdbe4d..6e6bd9e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -255,17 +255,6 @@ AC_SUBST(WCHAR32)
AM_CONDITIONAL(OS2, test x$os2 = xtrue)
-AC_ARG_WITH(launchd, AS_HELP_STRING([--with-launchd], [Build with support for Apple's launchd (default: auto)]), [LAUNCHD=$withval], [LAUNCHD=auto])
-if test "x$LAUNCHD" = xauto; then
- unset LAUNCHD
- AC_CHECK_PROG(LAUNCHD, [launchd], [yes], [no], [$PATH$PATH_SEPARATOR/sbin])
-fi
-
-if test "x$LAUNCHD" = xyes ; then
- AC_DEFINE(HAVE_LAUNCHD, 1, [launchd support available])
- AC_DEFINE(TRANS_REOPEN, 1, [launchd support available])
-fi
-
AC_ARG_ENABLE(xthreads,
AC_HELP_STRING([--disable-xthreads],
[Disable Xlib support for Multithreading]),
diff --git a/src/OpenDis.c b/src/OpenDis.c
index 8de9556..e568a30 100644
--- a/src/OpenDis.c
+++ b/src/OpenDis.c
@@ -69,7 +69,6 @@ XOpenDisplay (
int j, k; /* random iterator indexes */
char *display_name; /* pointer to display name */
char *setup = NULL; /* memory allocated at startup */
- char *fullname = NULL; /* expanded name of display */
int iscreen; /* screen number */
xConnSetupPrefix prefix; /* prefix information */
int vendorlen; /* length of vendor string */
@@ -117,13 +116,17 @@ XOpenDisplay (
return(NULL);
}
+ if ((dpy->display_name = strdup(display_name)) == NULL) {
+ OutOfMemory(dpy);
+ return(NULL);
+ }
+
/*
* Call the Connect routine to get the transport connection object.
- * If NULL is returned, the connection failed. The connect routine
- * will set fullname to point to the expanded name.
+ * If NULL is returned, the connection failed.
*/
- if(!_XConnectXCB(dpy, display, &fullname, &iscreen)) {
+ if(!_XConnectXCB(dpy, display, &iscreen)) {
/* Try falling back on other transports if no transport specified */
const char *slash = strrchr(display_name, '/');
if(slash == NULL) {
@@ -135,14 +138,13 @@ XOpenDisplay (
if(buf) {
for(s = protocols; buf && *s; s++) {
snprintf(buf, buf_size, "%s/%s", *s, display_name);
- if(_XConnectXCB(dpy, buf, &fullname, &iscreen))
+ if(_XConnectXCB(dpy, buf, &iscreen))
goto fallback_success;
}
Xfree(buf);
}
}
- dpy->display_name = fullname;
OutOfMemory(dpy);
return NULL;
}
@@ -152,7 +154,6 @@ fallback_success:
* Initialize pointers to NULL so that XFreeDisplayStructure will
* work if we run out of memory before we finish initializing.
*/
- dpy->display_name = fullname;
dpy->keysyms = (KeySym *) NULL;
dpy->modifiermap = NULL;
dpy->lock_meaning = NoSymbol;
@@ -314,7 +315,7 @@ fallback_success:
if (!mask)
{
fprintf (stderr, "Xlib: connection to \"%s\" invalid setup\n",
- fullname);
+ dpy->display_name);
OutOfMemory(dpy);
return (NULL);
}
@@ -678,8 +679,7 @@ void _XFreeDisplayStructure(Display *dpy)
Xfree ((char *)dpy->pixmap_format);
}
- if (dpy->display_name)
- Xfree (dpy->display_name);
+ free(dpy->display_name);
if (dpy->vendor)
Xfree (dpy->vendor);
diff --git a/src/Xxcbint.h b/src/Xxcbint.h
index c819162..1fa1a4d 100644
--- a/src/Xxcbint.h
+++ b/src/Xxcbint.h
@@ -40,7 +40,7 @@ typedef struct _X11XCBPrivate {
/* xcb_disp.c */
-int _XConnectXCB(Display *dpy, _Xconst char *display, char **fullnamep, int *screenp);
+int _XConnectXCB(Display *dpy, _Xconst char *display, int *screenp);
void _XFreeX11XCBStructure(Display *dpy);
#endif /* XXCBINT_H */
diff --git a/src/xcb_disp.c b/src/xcb_disp.c
index 690a603..0fa40de 100644
--- a/src/xcb_disp.c
+++ b/src/xcb_disp.c
@@ -54,11 +54,10 @@ void XSetAuthorization(char *name, int namelen, char *data, int datalen)
_XUnlockMutex(_Xglobal_lock);
}
-int _XConnectXCB(Display *dpy, _Xconst char *display, char **fullnamep, int *screenp)
+int _XConnectXCB(Display *dpy, _Xconst char *display, int *screenp)
{
char *host;
int n = 0;
- int len;
xcb_connection_t *c;
dpy->fd = -1;
@@ -69,20 +68,7 @@ int _XConnectXCB(Display *dpy, _Xconst char *display, char **fullnamep, int *scr
if(!xcb_parse_display(display, &host, &n, screenp))
return 0;
-
- len = strlen(host) + (1 + 20 + 1 + 20 + 1);
- *fullnamep = Xmalloc(len);
- if (!*fullnamep) {
- free(host);
- return 0;
- }
-
-#ifdef HAVE_LAUNCHD
- if(strncmp(host, "/tmp/launch", 11) == 0)
- snprintf(*fullnamep, len, "%s:%d", host, n);
- else
-#endif
- snprintf(*fullnamep, len, "%s:%d.%d", host, n, *screenp);
+ /* host and n are unused, but xcb_parse_display requires them */
free(host);
_XLockMutex(_Xglobal_lock);
--
1.7.0.4
More information about the xorg-devel
mailing list