On Thu 22 Nov 2007 at 23:13:08 +0100, Matthew D. Fuller via RT wrote:
> On Sun, Aug 12, 2007 at 08:23:20PM +0200 I heard the voice of
> ?? ?? via RT, and lo! it spake thus:
> >
> > to reproduce this, 
> >   1. run ctwmrc with attched .ctwmrc
> >   2. then get no-title bar window popup
> >        easiest way is using Firefox, go basic authenticated page
> >        then enable to get password popup
> >   3. click button to close popup (OK / Cancel /etc/)
> >   4. ... then ctwm crash, get core.
> 
> I get a slightly different trace.  Whether that's due to code changes,
> or corruption from signal handlers, I don't know.
> 
> Mine is bombing out in events.c:2681, which is:
> 
>     XTranslateCoordinates(dpy, Tmp_win->vs->window, [...]
> 
> Examination of the core in gdb shows that Tmp_win->vs is NULL, hence
> the SEGV.  The attached patch seems to prevent the crash here, but I
> have no idea whether it's a correct fix, or just invalid in a
> non-crashing way; it's very much shotgunned.  Olaf may know better?

I just found I had overlooked this mail in my inbox.  I'm looking at the
code to refresh my memory.

It appears that Tmp_win->vs is supposed to be non-NULL pretty much all
the time that a window is actually mapped in some virtual screen (and
point to that virtual screen).

In this context, a virtual screen corresponds to a physical monitor, and
if you have several physical monitors arranged with Xinerama, ctwm has a
virtual screen for each (while X does its best to do the opposite, if I
understand correctly, and present them all together as a single
display).

This code deals with the user dragging a window outside of the bounds of
the current virtual screen, and ctwm checks if it should move the window
to a different virtual screen.

(A tricky detail with virtual screens is this: each virtual screens must
display a different workspace, because each window can be shown on only
1 virtual screen. But there are also windows that are present in
multiple workspaces, and if one window happens to be in multiple visible
workspaces, it can still be shown in only one of them. And if it vanishes
from one, it must reappear in another. This makes handling the ->vs
pointer somewhat, eh, tricky).

So I do think that Tmp_win->vs == NULL is indeed a bug. Somehow ctwm
seems to think the window isn't on-screen.

Your patch is probably not a bad workaround, especially if there are no
multiple virtual screens around.

Personally my first workaround would probably be to check if it is NULL,
and if so, get out of the whole if-clause, because it is uncertain in
which virtual screen the window is anyway. Something like (untested):

        if ((xl < 0 || yt < 0 || xl > Scr->rootw || yt > Scr->rooth) &&
            Tmp_win->vs != NULL) {

(and looking at it now, instead of Scr->rootw and ->rooth, it should
probably use something else to better express that we're looking at a
virtual screen's width and height)

Maybe I'll have time to look at this in more detail later.

> Matthew Fuller     (MF4839)   |  [EMAIL PROTECTED]
-Olaf.
-- 
___ Olaf 'Rhialto' Seibert      -- You author it, and I'll reader it.
\X/ rhialto/at/xs4all.nl        -- Cetero censeo "authored" delendum esse.

Reply via email to