GNOME.org
 

Re: SSH ForwardX11 mc crash (still)

Hi,

On Wed, 2005-05-11 at 17:08, Leonard den Ottolander wrote:
> The SSH X11Forward crash is still existent in current CVS (2005-05-11).
> Maybe you haven't tried a correct test case. OpenSSH >= 3.8.1.
> 
> /etc/ssh/ssh_config should contain
> ForwardX11 yes
> ForwardX11Trusted no
> 
> and /etc/ssh/sshd_config should contain
> X11Forwarding yes
> 
> $ ssh localhost
> $ mc
> press a non alphanumeric key (f.e. arrow down)
> 
> Boom. BadWindow (invalid Window parameter).

I mistakenly assumed this patch was committed to MC_4_6_1_PRE.
Backported Roland's fix to the PRE tree. This seems to work. The above
issue is no longer reproducible with this patch applied. Can I commit
it? Can we have a release now?

Leonard.

-- 
mount -t life -o ro /dev/dna /genetic/research


--- src/Makefile.am.000	2004-11-24 00:04:18.000000000 +0100
+++ src/Makefile.am	2005-03-10 10:44:36.000000000 +0100
@@ -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
+	widget.h win.c win.h wtools.c wtools.h unixcompat.h		\
+	x11conn.h x11conn.c
 
 if CHARSET
 mc_SOURCES = $(SRCS) $(CHARSET_SRC)
--- src/Makefile.in.000	2005-05-11 16:24:55.000000000 +0200
+++ src/Makefile.in	2005-05-15 13:22:04.000000000 +0200
@@ -237,7 +237,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
+	widget.h win.c win.h wtools.c wtools.h unixcompat.h		\
+	x11conn.h x11conn.c
 
 @CHARSET_FALSE mc_SOURCES = $(SRCS)
 
@@ -281,7 +282,8 @@ am__mc_SOURCES_DIST = achown.c achown.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 \
-	charsets.c charsets.h selcodepage.c selcodepage.h
+	unixcompat.h x11conn.h x11conn.c charsets.c charsets.h \
+	selcodepage.c selcodepage.h
 am__objects_1 = achown.$(OBJEXT) background.$(OBJEXT) boxes.$(OBJEXT) \
 	chmod.$(OBJEXT) chown.$(OBJEXT) cmd.$(OBJEXT) color.$(OBJEXT) \
 	command.$(OBJEXT) complete.$(OBJEXT) cons.handler.$(OBJEXT) \
@@ -299,7 +301,7 @@ am__objects_1 = achown.$(OBJEXT) backgro
 	subshell.$(OBJEXT) textconf.$(OBJEXT) tree.$(OBJEXT) \
 	treestore.$(OBJEXT) user.$(OBJEXT) util.$(OBJEXT) \
 	utilunix.$(OBJEXT) view.$(OBJEXT) widget.$(OBJEXT) \
-	win.$(OBJEXT) wtools.$(OBJEXT)
+	win.$(OBJEXT) wtools.$(OBJEXT) x11conn.$(OBJEXT)
 am__objects_2 = charsets.$(OBJEXT) selcodepage.$(OBJEXT)
 @CHARSET_TRUE am_mc_OBJECTS = $(am__objects_1) $(am__objects_2)
 @CHARSET_FALSE am_mc_OBJECTS = $(am__objects_1)
@@ -388,7 +390,8 @@ am__depfiles_maybe = depfiles
 @AMDEP_TRUE@	./$(DEPDIR)/treestore.Po ./$(DEPDIR)/user.Po \
 @AMDEP_TRUE@	./$(DEPDIR)/util.Po ./$(DEPDIR)/utilunix.Po \
 @AMDEP_TRUE@	./$(DEPDIR)/view.Po ./$(DEPDIR)/widget.Po \
- AMDEP_TRUE@	./$(DEPDIR)/win.Po ./$(DEPDIR)/wtools.Po
+ AMDEP_TRUE@	./$(DEPDIR)/win.Po ./$(DEPDIR)/wtools.Po \
+ AMDEP_TRUE@	./$(DEPDIR)/x11conn.Po
 COMPILE = $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) \
 	$(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS)
 CCLD = $(CC)
@@ -538,6 +541,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote  /$(DEPDIR)/widget Po am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote  /$(DEPDIR)/win Po am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote  /$(DEPDIR)/wtools Po am__quote@
+ AMDEP_TRUE@@am__include@ @am__quote  /$(DEPDIR)/x11conn Po am__quote@
 
 .c.o:
 @am__fastdepCC_TRUE@	if $(COMPILE) -MT $@ -MD -MP -MF "$(DEPDIR)/$*.Tpo" \
--- src/key.c.000	2005-03-25 10:32:24.000000000 +0100
+++ src/key.c	2005-03-25 10:38:24.000000000 +0100
@@ -22,14 +22,15 @@
    Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */
 
 #include <config.h>
-#include <stdio.h>
-#include <sys/types.h>
-#include <string.h>
-#ifdef HAVE_UNISTD_H
-#   include <unistd.h>
-#endif
+
 #include <ctype.h>
 #include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <sys/types.h>
+#include <unistd.h>
 
 #include "global.h"
 #include "tty.h"
@@ -44,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__
@@ -405,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);
@@ -460,22 +422,24 @@ init_key_x11 (void)
 
 /* This has to be called before slang_init or whatever routine
    calls any define_sequence */
-void init_key (void)
+void
+init_key (void)
 {
     const char *term = getenv ("TERM");
-    
+
     /* This has to be the first define_sequence */
     /* So, we can assume that the first keys member has ESC */
     define_sequences (mc_default_keys);
 
     /* Terminfo on irix does not have some keys */
-    if (term && 
-        ((!strncmp (term, "iris-ansi", 9)) || (!strncmp (term, "xterm", 5))))
+    if (term
+	&& (!strncmp (term, "iris-ansi", 9) || !strncmp (term, "xterm", 5)
+	    || !strncmp (term, "rxvt", 4)))
 	define_sequences (xterm_key_defines);
 
     /* load some additional keys (e.g. direct Alt-? support) */
-    load_xtra_key_defines();
-    
+    load_xtra_key_defines ();
+
 #ifdef __QNX__
     if (term && strncmp (term, "qnx", 3) == 0) {
 	/* Modify the default value of use_8th_bit_as_meta: we would
@@ -493,17 +457,16 @@ void init_key (void)
 	 */
 	use_8th_bit_as_meta = 0;
     }
-#endif /* __QNX__ */
+#endif				/* __QNX__ */
 
 #ifdef HAVE_TEXTMODE_X11_SUPPORT
     init_key_x11 ();
-#endif /* HAVE_TEXTMODE_X11_SUPPORT */
+#endif				/* HAVE_TEXTMODE_X11_SUPPORT */
 
     /* Load the qansi-m key definitions 
        if we are running under the qansi-m terminal */
-    if ((term) && (strncmp (term, "qansi-m", 7) == 0))
-    {
-       define_sequences(qansi_key_defines);
+    if ((term) && (strncmp (term, "qansi-m", 7) == 0)) {
+	define_sequences (qansi_key_defines);
     }
 }
 
@@ -1320,13 +1283,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;
@@ -1430,14 +1388,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
 }
--- src/x11conn.c.000	2005-05-15 13:03:06.000000000 +0200
+++ src/x11conn.c	2005-04-06 20:04:43.000000000 +0200
@@ -0,0 +1,233 @@
+/*
+   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.
+ */
+
+/*
+   !!! WARNING !!!
+
+   This code uses setjmp() and longjmp(). Before you modify _anything_
+   here, please read the relevant sections of the C standard.
+*/
+
+#include <config.h>
+
+#ifndef HAVE_TEXTMODE_X11_SUPPORT
+typedef int dummy;		/* C99 forbids empty compilation unit */
+#else
+
+#include <setjmp.h>
+
+#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
+
+static gboolean handlers_installed = FALSE;
+
+/* 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; /* FIXME: get a better name */
+static gboolean longjmp_allowed = FALSE;
+
+/*** file private functions ********************************************/
+
+static int x_io_error_handler (Display *dpy)
+{
+    (void) dpy;
+
+    lost_connection = TRUE;
+    if (longjmp_allowed) {
+	longjmp_allowed = FALSE;
+	longjmp(x11_exception, 1);
+    }
+    return 0;
+}
+
+static int x_error_handler (Display *dpy, XErrorEvent *ee)
+{
+    (void) ee;
+    (void) func_XCloseDisplay (dpy);
+    return x_io_error_handler (dpy);
+}
+
+static void install_error_handlers(void)
+{
+    if (handlers_installed) return;
+
+    (void) func_XSetErrorHandler (x_error_handler);
+    (void) func_XSetIOErrorHandler (x_io_error_handler);
+    handlers_installed = TRUE;
+}
+
+static gboolean x11_available(void)
+{
+#ifdef HAVE_GMODULE
+    gchar *x11_module_fname;
+
+    if (lost_connection)
+	return FALSE;
+
+    if (x11_module != NULL)
+	return TRUE;
+
+    x11_module_fname = g_module_build_path (NULL, "X11");
+
+    if (x11_module_fname == NULL)
+	return FALSE;
+
+    x11_module = g_module_open (x11_module_fname, G_MODULE_BIND_LAZY);
+    g_free (x11_module_fname);
+
+    if (x11_module == NULL)
+	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;
+
+    install_error_handlers();
+    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
+    install_error_handlers();
+    return !(lost_connection);
+#endif
+}
+
+/*** public functions **************************************************/
+
+Display *mc_XOpenDisplay (const char *displayname)
+{
+    Display *retval;
+
+    if (x11_available()) {
+	if (setjmp(x11_exception) == 0) {
+	    longjmp_allowed = TRUE;
+	    retval = func_XOpenDisplay (displayname);
+	    longjmp_allowed = FALSE;
+	    return retval;
+	}
+    }
+    return NULL;
+}
+
+int mc_XCloseDisplay (Display *display)
+{
+    int retval;
+
+    if (x11_available()) {
+	if (setjmp(x11_exception) == 0) {
+	    longjmp_allowed = TRUE;
+	    retval = func_XCloseDisplay (display);
+	    longjmp_allowed = FALSE;
+	    return retval;
+	}
+    }
+    return 0;
+}
+
+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)
+{
+    Bool retval;
+
+    if (x11_available()) {
+	if (setjmp(x11_exception) == 0) {
+	    longjmp_allowed = TRUE;
+	    retval = func_XQueryPointer (display, win, root_return,
+		child_return, root_x_return, root_y_return,
+		win_x_return, win_y_return, mask_return);
+	    longjmp_allowed = FALSE;
+	    return retval;
+	}
+    }
+    *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;
+}
+
+#endif /* HAVE_TEXTMODE_X11_SUPPORT */
--- src/x11conn.h.000	2005-05-15 13:03:12.000000000 +0200
+++ src/x11conn.h	2005-05-15 13:03:12.000000000 +0200
@@ -0,0 +1,21 @@
+#ifndef MC_X11CONN_H
+#define MC_X11CONN_H
+
+/*
+   This module provides support for some X11 functions.  The functions
+   are loaded dynamically if GModule is available, and statically if
+   not.  X11 session handling is somewhat robust.  If there is an X11
+   error or a connection error, all further traffic to the X server
+   will be suppressed, and the functions will return reasonable default
+   values.
+*/
+
+#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
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]