Subject: fvwm seg fault with grayscale xpm and ColorLimit [patch]

From: Kevin Ryde <[email-masked]>
Date: 09 Jan 2000 09:52:56 +1000

[Cc. followups please, I'm not subscribed to fvwm-workers]

On the Debian packaged fvwm 2.2.4 I get a seg fault at fvwm startup
when using the attached mini.lambda.xpm in a menu entry and when
ColorLimit is non-zero.  As, for example, with with the attached
.fvwm2rc.  The image, incidentally, is from the Debian guile1.3
package.

I think the problem is in libs/Picture.c where color_reduce_pixmap and
c200_substitute_color assume the XpmColor c_color field is non-NULL.
This particular image has only a grayscale defined (black actually),
and ends up with c_color NULL.

The attached patch sets c_color from one of the other fields if it
isn't already set, the idea being to then do the color limiting on
whatever gray is in the image.

        cd fvwm-2.2.4/libs
        patch <Picture.c.diff

I tried this on a recent development snapshot and it has the same
problem, and the patch applies successfully to fix it there too.

I only use fvwm in pretty unsophisticated ways, so someone who knows
more about the image handling would want to check this change.

---

From: Dominik Vogt <[email-masked]>
Date: Sun, 9 Jan 2000 13:22:48 +0100

> # .fvwm2rc loading mini.lambda.xpm with a ColorLimit in force
> 
> ColorLimit 9
> 
> AddToMenu /Foo
> + "Foo%mini.lambda.xpm%" Exec exec xterm
> 
> Mouse 1   R  A  Menu /Foo

Thanks, I have applied the patch (with a modification so that it
doesn't leak memory).

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: Kevin Ryde <[email-masked]>
Date: 10 Jan 2000 06:51:04 +1000

> Thanks, I have applied the patch (with a modification so that it

Are you sure the extra free() is right?  I think XpmFreeXpmImage()
will free each of the colour pointers it finds non-NULL.  (I'm looking
at lib/Image.c and lib/Attrib.c in the xpm sources.)  Also, c_color is
needed back in LoadPicture() for the XpmCreatePixmapFromXpmImage().

-- 
Cc. followups please.

---

From: Dan Espen <[email-masked]>
Date: Sun, 09 Jan 2000 16:18:22 -0500

> needed back in LoadPicture() for the XpmCreatePixmapFromXpmImage().

Doing any kind of separate malloc seems to me to be a bad idea.
I plan to commit new logic for this, something like:

    color_table_ptr = image->colorTable; /* start of xpm color table */
    for(i=0; i<image->ncolors; i++) {   /* all colors in the xpm */
      /* Theres an array for this in the xpm library, but it doesn't
         appear to be part of the API.  Too bad. dje 01/09/00 */
      char **visual_color;
      if (color_table_ptr->c_color) {
        visual_color = &color_table_ptr->c_color;
      } else if (color_table_ptr->g_color) {
        visual_color = &color_table_ptr->g_color;
      } else if (color_table_ptr->g4_color) {
        visual_color = &color_table_ptr->g4_color;
      } else {                          /* its got to be one of these */
        visual_color = &color_table_ptr->m_color;
      }
      c200_substitute_color(visual_color,color_limit);
      color_table_ptr +=1;              /* counter for loop */
    }                                   /* end all colors in xpm */

--
Dan Espen
444 Hoes Lane  Room RRC 1C-214           E-mail: [email-masked]
Piscataway, NJ 08854                     Phone: (732) 699-5570
--
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: Kevin Ryde <[email-masked]>
Date: 10 Jan 2000 08:42:53 +1000

>     }                                   /* end all colors in xpm */

That's a good idea.  How about skipping c200_substitute_color() if all
of c/g/g4/m_color are NULL?  It's probably valid to have an xpm with
only an "s" symbolic colour name (though fvwm couldn't be expected to
do anything useful with that colour).

---

From: Dan Espen <[email-masked]>
Date: Sun, 09 Jan 2000 18:30:12 -0500

> do anything useful with that colour).

I don't know if its valid or not.  (having no actual color).

If I understand what a symbolic color is, the xpm library wouldn't
do anything with the color line either.

I thought I'd look at the XPM logic a little closer,  or see if anyone
on the  list  had more information.   I  have the printed  XPM docs at
work, I thought I'd check that too.

I really wish there was an array of these "c","g","g4" things so this
logic wouldn't be so susceptible to the current XPM implementation.

I know its only one extra test...