That looks OK to me.  

                        Mark.


On Fri, 28 Feb 2003, Kevin Brosius wrote:

> 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.)
> > > > > > > >
> > > > > > > > --
> > > > > > > > Kevin

_______________________________________________
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel

Reply via email to