X11 connection problems

[Roland Illig <roland illig gmx de>]

Hi all,

I think I have solved the X11 connection problems with mc. Can you 
please review this patch, and tell me if it solves the so-called 
"OpenSSH bug"?

The file x11conn.c is my current development version, so please redirect 
stderr to some file for logging.

The testcase that did not work before, but now does, is the following.

1. Inside an X session, start a terminal emulator.
   Inside the terminal emulator, start GNU screen.
   Inside GNU screen, start the Midnight Commander.

   $ screen
   $ mc 2>mc.log

2. Detach GNU screen.

   (Press C-a d)

3. Shut down the X server.

4. On a console, reattach the GNU screen with mc inside.

   $ screen -r

5. Press some keys within mc.

   Before the patch, mc terminated here.
   After the patch it continues running.

Roland

/*
   X11 support for the Midnight Commander.

   Copyright (C) 2005 The Free Software Foundation

   Written by:
   Roland Illig <roland illig gmx de>, 2005.

   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 2 of the License, or
   (at your option) any later version.

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program; if not, write to the Free Software
   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
 */

/*
   This module provides support for some X11 functions. The functions are
   loaded dynamically if GModule is available, and statically if not.
*/

#include <config.h>

#include <setjmp.h>		/* ugly, but there's no other way */
#include <stdio.h>

#ifndef HAVE_TEXTMODE_X11_SUPPORT
typedef int dummy;		/* C99 forbids empty compilation unit */
#else

#include <X11/Xlib.h>

#include "../src/global.h"

#ifdef HAVE_GMODULE
#  include <gmodule.h>
#endif

#include "x11conn.h"

/*** file scope type declarations **************************************/

typedef int (*mc_XErrorHandler_callback) (Display *, XErrorEvent *);
typedef int (*mc_XIOErrorHandler_callback) (Display *);

/*** file scope variables **********************************************/

#ifdef HAVE_GMODULE

static Display * (*func_XOpenDisplay) (_Xconst char *);
static int (*func_XCloseDisplay) (Display *);
static mc_XErrorHandler_callback (*func_XSetErrorHandler)
	(mc_XErrorHandler_callback);
static mc_XIOErrorHandler_callback (*func_XSetIOErrorHandler)
	(mc_XIOErrorHandler_callback);
static Bool (*func_XQueryPointer) (Display *, Window, Window *, Window *,
	int *, int *, int *, int *, unsigned int *);

static GModule *x11_module;

#else

#define func_XOpenDisplay	XOpenDisplay
#define func_XCloseDisplay	XCloseDisplay
#define func_XSetErrorHandler	XSetErrorHandler
#define func_XSetIOErrorHandler	XSetIOErrorHandler
#define func_XQueryPointer	XQueryPointer

#endif

/* This flag is set as soon as an X11 error is reported. Usually that
 * means that the DISPLAY is not available anymore. We do not try to
 * reconnect, as that would violate the X11 protocol. */
static gboolean lost_connection = FALSE;
static jmp_buf x11_exception;
static gboolean longjmp_allowed = FALSE;

/*** file private functions ********************************************/

static gboolean x11_available(void)
{
#ifdef HAVE_GMODULE
    gchar *x11_module_fname;
#endif

fprintf(stderr, "x11_available: lost_connection=%d\n", lost_connection);

#ifdef HAVE_GMODULE
    if (lost_connection)
	return FALSE;

    if (x11_module)
	return TRUE;

    x11_module_fname = g_module_build_path (NULL, "X11");

    if (!x11_module_fname)
	return FALSE;

    x11_module = g_module_open (x11_module_fname, G_MODULE_BIND_LAZY);
    g_free (x11_module_fname);

    if (!x11_module)
	return FALSE;

    if (!g_module_symbol (x11_module, "XOpenDisplay", (void *) &func_XOpenDisplay))
	goto cleanup;
    if (!g_module_symbol (x11_module, "XCloseDisplay", (void *) &func_XCloseDisplay))
	goto cleanup;
    if (!g_module_symbol (x11_module, "XQueryPointer", (void *) &func_XQueryPointer))
	goto cleanup;
    if (!g_module_symbol (x11_module, "XSetErrorHandler", (void *) &func_XSetErrorHandler))
	goto cleanup;
    if (!g_module_symbol (x11_module, "XSetIOErrorHandler", (void *) &func_XSetIOErrorHandler))
	goto cleanup;
    return TRUE;

cleanup:
    func_XOpenDisplay = 0;
    func_XCloseDisplay = 0;
    func_XQueryPointer = 0;
    func_XSetErrorHandler = 0;
    func_XSetIOErrorHandler = 0;
    g_module_close (x11_module);
    x11_module = NULL;
    return FALSE;
#else
    return !(lost_connection);
#endif
}

static int x_error_handler (Display *dpy, XErrorEvent *ee)
{
    (void) dpy;
    (void) ee;

    lost_connection = TRUE;
fprintf(stderr, "x_error_handler (longjmp_allowed=%d)\n", longjmp_allowed);
    if (longjmp_allowed) {
	longjmp_allowed = FALSE;
	longjmp(x11_exception, 1);
    }
    return 0;
}

static int x_io_error_handler (Display *dpy)
{
    (void) dpy;

    lost_connection = TRUE;
fprintf(stderr, "x_io_error_handler (longjmp_allowed=%d)\n", longjmp_allowed);
    if (longjmp_allowed) {
	longjmp_allowed = FALSE;
	longjmp(x11_exception, 1);
    }
    return 0;
}

/*** public functions **************************************************/

Display *mc_XOpenDisplay (const char *displayname)
{
fprintf(stderr, "mc_XOpenDisplay\n");
    if (!x11_available())
	return NULL;
    (void) func_XSetErrorHandler (x_error_handler);
    (void) func_XSetIOErrorHandler (x_io_error_handler);
    if (setjmp(x11_exception) != 0)
	return NULL;
    longjmp_allowed = TRUE;
    return func_XOpenDisplay (displayname);
}

int mc_XCloseDisplay (Display *display)
{
fprintf(stderr, "mc_XCloseDisplay\n");
    if (!x11_available() || setjmp(x11_exception) != 0)
	return 0;
    longjmp_allowed = TRUE;
    return func_XCloseDisplay (display);
}

Bool mc_XQueryPointer (Display *display, Window win, Window *root_return,
	Window *child_return, int *root_x_return, int *root_y_return,
	int *win_x_return, int *win_y_return, unsigned int *mask_return)
{
fprintf(stderr, "mc_XQueryPointer\n");
    if (!x11_available() || setjmp(x11_exception) != 0) {
	*root_return = None;
	*child_return = None;
	*root_x_return = 0;
	*root_y_return = 0;
	*win_x_return = 0;
	*win_y_return = 0;
	*mask_return = 0;
	return False;
    }
    longjmp_allowed = TRUE;
    return func_XQueryPointer (display, win, root_return, child_return,
	root_x_return, root_y_return, win_x_return, win_y_return,
	mask_return);
}

#endif /* HAVE_TEXTMODE_X11_SUPPORT */

#ifndef MC_X11CONN_H
#define MC_X11CONN_H

#include <X11/Xlib.h>

extern Display *mc_XOpenDisplay (const char *);
extern int mc_XCloseDisplay (Display *);

extern Bool mc_XQueryPointer (Display *, Window, Window *, Window *, int *, int *, int *, int *, unsigned int *);

#endif

Index: src/Makefile.am
===================================================================
RCS file: /cvsroot/mc/mc/src/Makefile.am,v
retrieving revision 1.42
diff -u -p -r1.42 Makefile.am
--- src/Makefile.am	25 Jan 2005 22:55:19 -0000	1.42
+++ src/Makefile.am	25 Feb 2005 23:30:42 -0000
@@ -59,7 +59,8 @@ SRCS =	achown.c achown.h background.c ba
 	slint.c	subshell.c subshell.h textconf.c textconf.h		\
 	tree.c tree.h treestore.c treestore.h tty.h user.c user.h	\
 	util.c util.h utilunix.c view.c view.h vfsdummy.h widget.c	\
-	widget.h win.c win.h wtools.c wtools.h unixcompat.h
+	widget.h win.c win.h wtools.c wtools.h unixcompat.h		\
+	x11conn.h x11conn.c
 
 if CHARSET
 mc_SOURCES = $(SRCS) $(CHARSET_SRC)
Index: src/key.c
===================================================================
RCS file: /cvsroot/mc/mc/src/key.c,v
retrieving revision 1.78
diff -u -p -r1.78 key.c
--- src/key.c	8 Feb 2005 09:04:03 -0000	1.78
+++ src/key.c	25 Feb 2005 23:30:45 -0000
@@ -45,10 +45,7 @@
 #endif
 
 #ifdef HAVE_TEXTMODE_X11_SUPPORT
-#ifdef HAVE_GMODULE
-#include <gmodule.h>
-#endif /* HAVE_GMODULE */
-#include <X11/Xlib.h>
+#    include "x11conn.h"
 #endif
 
 #ifdef __linux__
@@ -406,52 +403,16 @@ define_sequences (key_define_t *kd)
 
 #ifdef HAVE_TEXTMODE_X11_SUPPORT
 
-#ifdef HAVE_GMODULE
-static int (*func_XCloseDisplay) (Display *);
-static Bool (*func_XQueryPointer) (Display *, Window, Window *, Window *,
-				   int *, int *, int *, int *,
-				   unsigned int *);
-
-static GModule *x11_module;
-#endif				/* HAVE_GMODULE */
-
 static Display *x11_display;
 static Window x11_window;
 
 static void
 init_key_x11 (void)
 {
-#ifdef HAVE_GMODULE
-    static Display *(*func_XOpenDisplay) (_Xconst char *);
-    gchar *x11_module_fname;
-#endif				/* HAVE_GMODULE */
-
     if (!getenv ("DISPLAY"))
 	return;
 
-#ifdef HAVE_GMODULE
-    x11_module_fname = g_module_build_path (NULL, "X11");
-
-    if (!x11_module_fname)
-	return;
-
-    x11_module = g_module_open (x11_module_fname, G_MODULE_BIND_LAZY);
-    g_free (x11_module_fname);
-
-    if (!x11_module)
-	return;
-
-    if (g_module_symbol
-	(x11_module, "XOpenDisplay", (void *) &func_XOpenDisplay)
-	&& g_module_symbol (x11_module, "XCloseDisplay",
-			    (void *) &func_XCloseDisplay)
-	&& g_module_symbol (x11_module, "XQueryPointer",
-			    (void *) &func_XQueryPointer)) {
-	x11_display = (*func_XOpenDisplay) (0);
-    }
-#else
-    x11_display = XOpenDisplay (0);
-#endif				/* HAVE_GMODULE */
+    x11_display = mc_XOpenDisplay (0);
 
     if (x11_display)
 	x11_window = DefaultRootWindow (x11_display);
@@ -1324,13 +1285,8 @@ get_modifier (void)
 	int win_x, win_y;
 	unsigned int mask;
 
-#ifdef HAVE_GMODULE
-	(*func_XQueryPointer) (x11_display, x11_window, &root, &child,
-			       &root_x, &root_y, &win_x, &win_y, &mask);
-#else
-	XQueryPointer (x11_display, x11_window, &root, &child, &root_x,
+	mc_XQueryPointer (x11_display, x11_window, &root, &child, &root_x,
 		       &root_y, &win_x, &win_y, &mask);
-#endif				/* HAVE_GMODULE */
 
 	if (mask & ShiftMask)
 	    result |= KEY_M_SHIFT;
@@ -1440,14 +1396,7 @@ void done_key ()
     s_dispose (select_list);
 
 #ifdef HAVE_TEXTMODE_X11_SUPPORT
-#ifdef HAVE_GMODULE
     if (x11_display)
-	(*func_XCloseDisplay) (x11_display);
-    if (x11_module)
-	g_module_close (x11_module);
-#else
-    if (x11_display)
-	XCloseDisplay (x11_display);
-#endif /* HAVE_GMODULE */
-#endif /* HAVE_TEXTMODE_X11_SUPPORT */
+	mc_XCloseDisplay (x11_display);
+#endif
 }