GNU bug report logs - #6855
24.0.50; Bug in tool bar label handling

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 6855 in the body.
You can then email your comments to 6855 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Johan Bockgård <bojohan <at> gnu.org>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.0.50; Bug in tool bar label handling
Date: Sat, 14 Aug 2010 14:04:25 +0200

There are some bugs in the handling of tool bar labels that can cause
Emacs to crash.



### gtkutil.c: update_frame_tool_bar ###

    char *label = SSDATA (PROP (TOOL_BAR_ITEM_LABEL));

Here we take string data out.



### keyboard.c: parse_tool_bar_item ###

      else if (EQ (key, QClabel))
        {
          /* `:label LABEL-STRING'.  */
          PROP (TOOL_BAR_ITEM_LABEL) = value;
          have_label = 1;
        }

But here we put an arbitrary object in.


...

  if (!have_label)

...
      char buf[64];
      EMACS_INT max_lbl = 2*tool_bar_max_label_size;
      Lisp_Object new_lbl;

      if (strlen (caption) < max_lbl && caption[0] != '\0')
        {
          strcpy (buf, caption);

tool-bar-max-label-size is a user variable, so this can mean a buffer
overflow.


...
      if (SCHARS (new_lbl) <= tool_bar_max_label_size)
        PROP (TOOL_BAR_ITEM_LABEL) = new_lbl;

If we came here but the branch is not taken, the label will be nil,
not a string.

Message #10 received at 6855-done <at> debbugs.gnu.org (full text, mbox):

From: Jan Djärv <jan.h.d <at> swipnet.se>
To: Johan Bockgård <bojohan <at> gnu.org>
Cc: 6855-done <at> debbugs.gnu.org
Subject: Re: bug#6855: 24.0.50; Bug in tool bar label handling
Date: Sun, 15 Aug 2010 10:20:05 +0200

Johan Bockgård skrev 2010-08-14 14.04:
>
> There are some bugs in the handling of tool bar labels that can cause
> Emacs to crash.
>
>
>
> ### gtkutil.c: update_frame_tool_bar ###
>
>      char *label = SSDATA (PROP (TOOL_BAR_ITEM_LABEL));
>
> Here we take string data out.
>
>
>
> ### keyboard.c: parse_tool_bar_item ###
>
>        else if (EQ (key, QClabel))
>          {
>            /* `:label LABEL-STRING'.  */
>            PROP (TOOL_BAR_ITEM_LABEL) = value;
>            have_label = 1;
>          }
>
> But here we put an arbitrary object in.
>

We kind of assume people do the sensible thing and put in strings.  It is the
same as for help and image.  If Emacs crashes because somebody didn't put
in a string, that is actually a good thing IMHO.  The error becomes very
apparent then.

>
> ...
>
>    if (!have_label)
>
> ...
>        char buf[64];
>        EMACS_INT max_lbl = 2*tool_bar_max_label_size;
>        Lisp_Object new_lbl;
>
>        if (strlen (caption)<  max_lbl&&  caption[0] != '\0')
>          {
>            strcpy (buf, caption);
>
> tool-bar-max-label-size is a user variable, so this can mean a buffer
> overflow.
>
>
> ...
>        if (SCHARS (new_lbl)<= tool_bar_max_label_size)
>          PROP (TOOL_BAR_ITEM_LABEL) = new_lbl;
>
> If we came here but the branch is not taken, the label will be nil,
> not a string.
>

I have checked in a fix for those two.

Thanks,

	Jan D.

Message #13 received at 6855 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: 6855 <at> debbugs.gnu.org
Cc: jan.h.d <at> swipnet.se
Subject: Re: bug#6855: 24.0.50; Bug in tool bar label handling
Date: Sun, 15 Aug 2010 10:51:37 +0200

Jan Djärv <jan.h.d <at> swipnet.se> writes:

> We kind of assume people do the sensible thing and put in strings.  It is the
> same as for help and image.  If Emacs crashes because somebody didn't put
> in a string, that is actually a good thing IMHO.  The error becomes very
> apparent then.

I don't agree.  Emacs should be robust against type mismatches, crashing
is the worst possible reaction.

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Message #16 received at 6855 <at> debbugs.gnu.org (full text, mbox):

From: Jan Djärv <jan.h.d <at> swipnet.se>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: 6855 <at> debbugs.gnu.org
Subject: Re: bug#6855: 24.0.50; Bug in tool bar label handling
Date: Sun, 15 Aug 2010 12:21:37 +0200

Andreas Schwab skrev 2010-08-15 10.51:
> Jan Djärv<jan.h.d <at> swipnet.se>  writes:
>
>> We kind of assume people do the sensible thing and put in strings.  It is the
>> same as for help and image.  If Emacs crashes because somebody didn't put
>> in a string, that is actually a good thing IMHO.  The error becomes very
>> apparent then.
>
> I don't agree.  Emacs should be robust against type mismatches, crashing
> is the worst possible reaction.

If the documentation states that one should use STRING, and somebody puts in 
nil or a lambda expression or a symbol, that is a usage error.  Being robust 
against this kind of error by ignoring the faulty input just hides the error 
and makes people think it is OK to misuse things.  Better then to crash, that 
way action is usually taken at once.  Hidden errors can linger for years...

	Jan D.

Message #19 received at 6855 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Jan Djärv <jan.h.d <at> swipnet.se>
Cc: 6855 <at> debbugs.gnu.org
Subject: Re: bug#6855: 24.0.50; Bug in tool bar label handling
Date: Sun, 15 Aug 2010 12:37:29 +0200

Jan Djärv <jan.h.d <at> swipnet.se> writes:

> If the documentation states that one should use STRING, and somebody puts
> in nil or a lambda expression or a symbol, that is a usage error.

Sure.  But crashing is a bug.

> Better then to crash

No, never.

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Message #22 received at 6855 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jan Djärv <jan.h.d <at> swipnet.se>
Cc: 6855 <at> debbugs.gnu.org, schwab <at> linux-m68k.org
Subject: Re: bug#6855: 24.0.50; Bug in tool bar label handling
Date: Sun, 15 Aug 2010 07:41:49 -0400

> Date: Sun, 15 Aug 2010 12:21:37 +0200
> From: Jan Djärv <jan.h.d <at> swipnet.se>
> Cc: 6855 <at> debbugs.gnu.org
> 
> If the documentation states that one should use STRING, and somebody puts in 
> nil or a lambda expression or a symbol, that is a usage error.  Being robust 
> against this kind of error by ignoring the faulty input just hides the error 
> and makes people think it is OK to misuse things.  Better then to crash, that 
> way action is usually taken at once.  Hidden errors can linger for years...

Crash or hide are not the only alternatives.  You can signal an error,
for instance.  Sometimes doing so is not a good idea, like in the
middle of redisplay (because displaying the error message reenters
redisplay again, and you have an infinite loop on your hands).  For
these situations, the solution is to display something prominent and
acutely visible instead of the invalid data, so that it stands out and
catches the user's eye.  For example, if the menu item is bad, display
something like "!!??GARBLED ITEM??!!" instead.

In general, I agree with Andreas: it is better not to crash, if we can
avoid that with a reasonable effort.

Message #25 received at 6855 <at> debbugs.gnu.org (full text, mbox):

From: MON KEY <monkey <at> sandpframing.com>
To: 6855 <at> debbugs.gnu.org
Cc: 6835 <at> debbugs.gnu.org, Andreas Schwab <schwab <at> linux-m68k.org>
Subject: bug#6855: 24.0.50; Bug in tool bar label handling
Date: Sun, 15 Aug 2010 19:45:31 -0400

> I don't agree.  Emacs should be robust against type mismatches,
> crashing is the worst possible reaction.

Unless of course that robustness is requested of `type-of', in which
case presumably Andreas waffles and TRT is to simply not use a type
macthing feature that may cause Emacs to crash, e.g. this recent
comment re bug#6835:

,----
|
| > `type-of' does consistently bring down my Emacs.
|
| Try without type-of.
|
| Andreas.
|
`---- http://lists.gnu.org/archive/html/bug-gnu-emacs/2010-08/msg00337.html

:-P

> Andreas.

--
/s_P\

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.