Subject: Freeing colours multiple times w/ colour sets

From: Dominik Vogt <[email-masked]>
Date: Mon, 12 Mar 2001 00:40:51 +0100

I noticed there is a problem in the colour allocation of menus and
probably other parts of fvwm.  With this config:

  MenuStyle * Foreground $[fg.cs0], Background $[bg.cs0]

The GetColor call will return the Pixel structures from colour set
0 without allocating any colour cells.  If the same line is issued
again, XFreeColors() is called for these colour cells and then
GetColor() is called again, returning the same colour cell again
and so on.  I'm not sure what will happen.  Probably nothing
because fvwm never allocated this colour cell - it was allocated
by FvwmTheme.  But I am not sure about the effects - fvwm might
even crash.

Bye

Dominik ^_^  ^_^

--
Dominik Vogt, [email-masked]
Reply-To: [email-masked]
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Dominik Vogt <[email-masked]>
Date: Mon, 12 Mar 2001 13:04:42 +0100

On Mon, Mar 12, 2001 at 10:22:33AM +0000, Tim Phipps wrote:
> [email-masked] wrote:
> Are you sure this is what happens? and `echo $[fg.cs0]' gives:
> [FVWM][Echo]:  rgb:ffff/ffff/0000

In a function, the pixel value is converted to a string before
using it.  From functions.c (expand_extended_var):

    case 0:
      /* fg.cs */
      pixel = Colorset[cs].fg;
      break;
    ...
    return pixel_to_color_string(dpy, Pcmap, pixel, target, False);

> so wouldn't the MenuStyle line above alloc the pixels it needs?

Yes and no.  If the "$[fg.cs<n>]" string is expanded before the
MenuStyle command is executed, there is no problem.  But if the
string is quoted so that it is used as the colour name, GetColor
does the substitution with the described limitation.  As far as I
remember this is intended for use by modules which will suffer
from the same problem as the menu code.
 
> fvwm won't crash but it will get an error from the server (which it
> ignores - fvwm.c line 2085). If fvwm is trying to free pixels it hasn't
> alloc'd we will have to fix this when FvwmTheme is moved into fvwm
> because it will be able to free the pixels and bad things will happen,
> maybe just wrong colors but maybe crashes.

Yes.  Currectly I guess that colours may be corrupted.  Consider
this:

 - Server running with 256 colours
 - Colour x is used in a colour set and allocated by FvwmTheme.
 - Same colour x is allocated exactly once by fvwm.
 - Issue "MenuStyle * $$[fg.cs0]"
 - Issue "MenuStyle * $$[fg.cs0]" again

The second MenuStyle command will free colour x which is still in
use somewhere else, for example in window titles.  If the colour
set changes now, the colour cell used in window hilighting will
change, right?

----------------------------- snip ----------------------------
*FvwmThemeColorset 0 fg rgb:1234/5678/9abc
ModuleSynchronous Timeout 5 FvwmTheme
HilightColor rgb:1234/5678/9abc red
MenuStyle * Foreground $$[fg.cs0]
MenuStyle * Foreground $$[fg.cs0]
SendToModule FvwmTheme ColorSet 0 Foreground blue
----------------------------- snip ----------------------------

Bye

Dominik ^_^  ^_^

--
Dominik Vogt, email: [email-masked]
LifeBits Aktiengesellschaft, Albrechtstr. 9, D-72072 Tuebingen
fon: ++49 (0) 7071/7965-0, fax: ++49 (0) 7071/7965-20
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Tim Phipps <[email-masked]>
Date: Mon, 12 Mar 2001 14:41:21 +0000

> from the same problem as the menu code.

Maybe the fix is for GetColor to alloc the color.

Cheers,
Tim.

---

From: Dominik Vogt <[email-masked]>
Date: Mon, 12 Mar 2001 16:32:05 +0100

> Maybe the fix is for GetColor to alloc the color.

I think that's the way to go.

Bye

Dominik ^_^  ^_^