mythtv-setup crashes when using remote X display (via ssh)

[anonymous]

I am trying to install the backend on a machine that has no local X server. The display is being forwarded to a MacOSX machine via SSH (-X -Y). This might be a bug in libxrandr.

Starting program: /home/gal/src/mythtv/mythtv/programs/mythtv-setup/mythtv-setup [Thread debugging using libthread_db enabled] [New Thread -1245915440 (LWP 21268)] Qt: gdb: -nograb added to command-line options.

Use the -dograb option to enforce grabbing.

Xlib: extension "XInputExtension" missing on display "localhost:10.0". Failed to get list of devices 2007-05-02 18:50:18.967 Using runtime prefix = /usr 2007-05-02 18:50:18.984 DPMS is not supported. 2007-05-02 18:50:19.280 New DB connection, total: 1 2007-05-02 18:50:19.305 Connected to database 'mythconverg' at host: localhost 2007-05-02 18:50:19.309 Total desktop dim: 1280x800, with 1 screen[s]. 2007-05-02 18:50:19.348 Using screen 0, 1280x778 at 0,22 2007-05-02 18:50:19.375 Current Schema Version: 1188

Program received signal SIGSEGV, Segmentation fault. [Switching to Thread -1245915440 (LWP 21268)] 0xb69a6454 in ?? () from /usr/lib/libXrandr.so.2 (gdb) bt #0 0xb69a6454 in ?? () from /usr/lib/libXrandr.so.2 #1 0x000052e9 in ?? () #2 0x08148d18 in ?? () #3 0x00000048 in ?? () #4 0xb608bb2c in ?? () from /usr/lib/libX11.so.6 #5 0x00000000 in ?? () (gdb) quit

[gal]

The cause is XRR not being checked before calling into it. The attached patch fixes the problem for me.

Index: DisplayResX.cpp
===================================================================
--- DisplayResX.cpp     (revision 13397)
+++ DisplayResX.cpp     (working copy)
@@ -121,7 +121,10 @@
 
     X11L;
     Window root = RootWindow(display, DefaultScreen(display));
-    XRRScreenConfiguration *cfg = XRRGetScreenInfo(display, root);
+    XRRScreenConfiguration *cfg = NULL;
+    int event_base, error_base;
+    if (XRRQueryExtension(display, &event_base, &error_base))
+       cfg =  XRRGetScreenInfo(display, root);
     if (!cfg)
     {
         if (display)

[tdegibson_at_gmail_dot_com]

Also seg fault from running mythtv-setup via vnc window... downgrading to libXrandr-1.1.1 from libXrandr-1.2.1 on Gentoo fixed seg fault but obviously not an ideal fix...

Tim

[Dibblah]

This patch is a similar fix. Fixes in the case where XRandR has been configured, but is no longer available as well.

[eloy@…]

The patch by Dibblah is bogus since it calls XRRQueryExtension() with two NULL pointers. If you look at the source for XRRQueryExtension() you'll see that the function provides return values via these two pointers. If the pointers are NULL then we have a nice crash, like what we're currently having in the 0.20-svn20070523-0.0 Debian packages maintained by Christian Marillat since he integrated this patch.

[anonymous]

eloy: You're completely wrong. XRRQueryExtension() can and should be called with two NULL pointers. You can first start by reading the man page, ​http://www.die.net/doc/linux/man/man3/xrrqueryextension.3.html

Then read the code. Those are called "out variables" or callback variables. If you don't care about the data coming to you, you set them equal to NULL and all is well.

[eloy@…]

Replying to anonymous:

eloy: You're completely wrong. XRRQueryExtension() can and should be called with two NULL pointers. You can first start by reading the man page, ​http://www.die.net/doc/linux/man/man3/xrrqueryextension.3.html

Then read the code. Those are called "out variables" or callback variables. If you don't care about the data coming to you, you set them equal to NULL and all is well.

anonymous: if you set to NULL the pointers to your "out variables" in the specific case of XRRQueryExtension() then nothing is well.

I read the code before claiming that the patch is bogus. It seems like you haven't read it, or if you read it you didn't understand the problem, so how about we read the code together? See:

​http://csourcesearch.net/package/kdrive/4.3.0/xc/lib/Xrandr/Xrandr.c

XRRQueryExtension(Display *dpy, int *event_basep, int *error_basep) is defined at line 339.

If the function parameters event_basep and error_basep are NULL then lines 344 and 345 will be writing something to address 0x00000000. I am sure you know that doing this is bad, and will cause a crash.

Trust me on this one - the patch is bogus; you can't pass NULL in the two pointers that XRRQueryExtension() receives.

This patch was applied to the MythTV Debian packages and had to be backed out because the backend was crashing as soon as it started. Debugging showed that it was crashing because of the problem that I describe above.

The correct way to call XRRQueryExtension() is to pass real, valid pointers, not NULL pointers.

[danielk]

(In [13766]) Fixes #3401. Applies slightly modified version of patch.

This just checks if we have the xrandr extension before using it. This fixes a segfault for people recompile their own X11 servers to exclude XRandR support after compiling MythTV, or when they have X11 installed on an OSX machine.

I added the dummy variables argued over in the ticket comments. While I don't think they are required by the API, an implementation of XRandR in wide release appears to require them in practice, whether this is a new bug in the implementation or a new bug in the API isn't really important; the code that works with this XRandR release is still correct according API and only one line longer.

[anonymous]

Will this get backported to -fixes?

[stuartm]

This needs backporting to -fixes, unless there are any objections I'll do it?

[dan@…]

Thanks Stuart!

[stuartm]

(In [14223]) Backports [13766] to -fixes.

Fixes a segfault with mythtv-setup when using X11 forwarding and XRandR is not available.

See #3401 for more detail.

Fixes #3401