Yeah, that's what it's trying to do. The problem is it doesn't check
the first child of root for InputOnly, only 2nd children on up. E17 has
an InputOnly window as the first child of root, which breaks xmag. I'd
suggest the following patch. It checks to make sure all children are
InputOutput and will only return one that is, or the root. (I suspect
that was the original intent.)
I've also included fixes for the null pointer dereferences exhibited
when the above code failed. What do you think?
Summary of change:
Fix case in xmag which would cause a BadMatch during a X_GetImage for
single child of root class InputOnly. Also do some null pointer
protection.
--
Kevin
Mark Vojkovich wrote:
>
>
> On Wed, 26 Feb 2003, Kevin Brosius wrote:
>
> > The background reports "Depth: 0" with xwininfo. That looks like a
> > problem.
> >
> > The x,y,... to the GetImage seem good, 1103,302,64,64.
>
> What does xwininfo say the "Class" is? It should be InputOnly.
> Depth 0 windows are InputOnly.
>
> Can you poke around in xmag's FindWindow? Maybe the logic
> in there is busted. I'm not really following what it's supposed
> to be doing. Looks like it starts at the root and works it's way
> towards the pointer (out of the screen) until the last InputOutput
> window, which is the only type it can grab from. I would guess it
> wants the frontmost InputOutput window under the pointer.
>
> Mark.
>
> >
> > --
> > Kevin
> >
> >
> > Mark Vojkovich wrote:
> > >
> > >
> > > Are there depth 32 windows or something? If not
> > > GetImageAndAttributes should call XGetImage on the
> > > root window. It looks like the checks in there are OK.
> > > Can you check the x,y,width,height to that first XGetImage
> > > in GetImageAndAttributes?
> > >
> > > Mark.
> > >
> > > On Tue, 25 Feb 2003, Kevin Brosius wrote:
> > >
> > > > Of course, that's not what causes the X_GetImage failure... That
> > > > happens here:
> > > >
> > > > in DragEH()
> > > >
> > > > case ButtonRelease: /* end drag mode */
> > > > if (event->xbutton.button == Button1) { /* get image */
> > > > /* Problem: You can't get bits with XGetImage outside of its
> > > > window.
> > > > * xmag will only do a GetImage on the actual window in
> > > > the case
> > > > * where the depth of the window does not match the depth
> > > > of
> > > > * the root window.
> > > > */
> > > > GetImageAndAttributes(FindWindow(event->xmotion.x_root,
> > > > event->xmotion.y_root),
> > > > event->xbutton.x_root,
> > > > event->xbutton.y_root,
> > > > srcWidth, srcHeight, data);
> > > >
> > > > the GetImage call above generates the BadMatch. The embedded
> > > > FindWindow() calls seem to succeed.
> > > >
> > > > Kevin Brosius wrote:
> > > > >
> > > > > Latest CVS xmag (yesterday, changelog at 948.) No, there's no overlay,
> > > > > this is on an ATI Mach64 at depth 16. I do think e17 uses a virtual
> > > > > root window however. Here's the problem:
> > > > >
> > > > > (gdb) r
> > > > > X Error of failed request: BadMatch (invalid parameter attributes)
> > > > > Major opcode of failed request: 73 (X_GetImage)
> > > > > Serial number of failed request: 2375
> > > > > Current serial number in output stream: 2375
> > > > >
> > > > > Program received signal SIGSEGV, Segmentation fault.
> > > > > 0x0804b48d in GetMinIntensity (data=0x805b5b8) at xmag.c:908
> > > > > (gdb) bt
> > > > > #0 0x0804b48d in GetMinIntensity (data=0x805b5b8) at xmag.c:908
> > > > > #1 0x0804b7cf in PopupNewScale (data=0x805b5b8) at xmag.c:975
> > > > > #2 0x0804a909 in DragEH (w=0x805fc38, closure=0x805b5b8,
> > > > > event=0xbffff460, continue_to_dispatch=0xbffff38f
> > > > > "\001`ôÿ¿8ü\005\bdf\005\b8·\005\b\a") at xmag.c:664
> > > > > #3 0x400accf4 in XtDispatchEventToWidget () from
> > > > > /usr/X11R6/lib/libXt.so.6
> > > > > #4 0x400ad767 in _XtDefaultDispatcher () from /usr/X11R6/lib/libXt.so.6
> > > > > #5 0x400ad944 in XtDispatchEvent () from /usr/X11R6/lib/libXt.so.6
> > > > > #6 0x400adea2 in XtAppMainLoop () from /usr/X11R6/lib/libXt.so.6
> > > > > #7 0x0804bd43 in main (argc=1, argv=0xbffff554) at xmag.c:1105
> > > > > #8 0x402204a2 in __libc_start_main () from /lib/libc.so.6
> > > > > (gdb) l
> > > > >
> > > > > static Pixel
> > > > > GetMinIntensity(hlPtr data)
> > > > > {
> > > > > XColor *colors = NULL, *mptr, *tptr;
> > > > > int i, ncolors;
> > > > >
> > > > > if (data->win_info.colormap == DefaultColormap(dpy, scr))
> > > > > return BlackPixel(dpy, scr);
> > > > > ncolors = Get_XColors(&data->win_info, &colors);
> > > > > mptr = tptr = colors; tptr++;
> > > > >
> > > > > 903 for (i=1; i<ncolors; i++) {
> > > > > 904 if ((int)Intensity(mptr) > (int)Intensity(tptr))
> > > > > 905 mptr = tptr;
> > > > > 906 tptr++;
> > > > > 907 }
> > > > > 908 return mptr->pixel;
> > > > > 909 }
> > > > > 910
> > > > > (gdb) p ncolors
> > > > > $1 = 0
> > > > >
> > > > > I pasted some extra lines from the function in question. Get_XColors
> > > > > returns 0 in my case, and since mptr was set to NULL already, the return
> > > > > segv's when hit (the for loop falls through.) What should happen in
> > > > > this case? Can we just return BlackPixel like the previous test?
> > > > >
> > > > > --
> > > > > Kevin
> > > > >
> > > > > Mark Vojkovich wrote:
> > > > > >
> > > > > >
> > > > > > I don't see how the window manager could be involved. How
> > > > > > current of CVS? The last thing in the CHANGELOG on my machine is
> > > > > > 862 and I don't see this problem.
> > > > > >
> > > > > > I think BadMatch can happen with GetImage only if the app
> > > > > > trys to grab outside of the window. I think xmag grabs on the
> > > > > > root window to avoid this, but can only do this when the depth
> > > > > > of the window in question is the root depth. You don't have
> > > > > > different depth windows do you (overlay, depth 32 windows).
> > > > > > It looks like it knows how to clamp to the window dimensions.
> > > > > >
> > > > > > Maybe you can get a backtrace with a debug xmag?
> > > > > >
> > > > > > Maybe it has something to do with RandR?
> > > > > >
> > > > > > Mark.
> > > > > >
> > > > > > On Mon, 24 Feb 2003, Kevin Brosius wrote:
> > > > > >
> > > > > > > I've noticed the following xmag segv with current CVS when trying to
> > > > > > > view part of the background in the development version of e (e17). Is
> > > > > > > this an xmag or a window manager problem? (Or both?)
> > > > > > >
> > > > > > >
> > > > > > > (gdb) r
> > > > > > > X Error of failed request: BadMatch (invalid parameter attributes)
> > > > > > > Major opcode of failed request: 73 (X_GetImage)
> > > > > > > Serial number of failed request: 668
> > > > > > > Current serial number in output stream: 668
> > > > > > >
> > > > > > > Program received signal SIGSEGV, Segmentation fault.
> > > > > > > 0x0804afbf in GetMinIntensity ()
> > > > > > >
> > > > > > > This only occurs when the start point of the xmag selection window is
> > > > > > > over the background. Clicking inside an application works fine, as does
> > > > > > > clicking inside but near an application edge which shows both
> > > > > > > application and background image magnification. (I can magnify the
> > > > > > > background as long as the click doesn't occur on it.)
> > > > > > >
> > > > > > > --
> > > > > > > KevinIndex: xmag.c
===================================================================
RCS file: /usr/local/src/cvsup-rep/xc/programs/xmag/xmag.c,v
retrieving revision 1.11
diff -u -r1.11 xmag.c
--- xmag.c 19 Jan 2003 04:44:45 -0000 1.11
+++ xmag.c 1 Mar 2003 02:59:43 -0000
@@ -579,12 +579,17 @@
{
XWindowAttributes wa;
Window findW = DefaultRootWindow(dpy), stopW, childW;
- XTranslateCoordinates(dpy, findW, findW,
- x, y, &x, &y, &stopW);
+
+ /* Setup for first window find */
+ stopW = findW;
+
while (stopW) {
XTranslateCoordinates(dpy, findW, stopW,
x, y, &x, &y, &childW);
findW = stopW;
+ /* If child is not InputOutput (for example, InputOnly) */
+ /* then don't continue, return the present findW which */
+ /* can be the root, or a root child of class InputOutput */
if (childW &&
XGetWindowAttributes(dpy, childW, &wa) &&
wa.class != InputOutput)
@@ -884,7 +889,11 @@
mptr = tptr;
tptr++;
}
- return mptr->pixel;
+ /* Null pointer protection */
+ if(mptr)
+ return mptr->pixel;
+ else
+ return WhitePixel(dpy, scr);
}
/*
@@ -905,7 +914,11 @@
mptr = tptr;
tptr++;
}
- return mptr->pixel;
+ /* Null pointer protection */
+ if(mptr)
+ return mptr->pixel;
+ else
+ return BlackPixel(dpy, scr);
}
