On Thu, 13 Jul 2006, Dominik Vogt wrote:

On Thu, Jul 13, 2006 at 02:55:24PM +0200, Viktor Griph wrote:
On Thu, 13 Jul 2006, Dominik Vogt wrote:

On Thu, Jul 13, 2006 at 09:25:09AM +0200, Viktor Griph wrote:
On Thu, 13 Jul 2006, Scott Smedley wrote:

Hi all,

I think there is a bug in FVWM's parameter expansion.
FVWM crashes with a simple command such as:

Echo $[0]

I am looking at this problem in GDB. The variable 'm', suddenly has a
huge value when I reach line 918 of fvwm/expand.c:

if (input[m] == ']')

Then I get a SEGV because this is an illegal memory access.

Can anyone else confirm this problem?


I can'tr make it crash with just Echo $[0]. However the following make it
crash 100%:

AddToFunc TestFunc I Echo $[0]
TestFunc $[0]

I'll investigate that further after breakfast to see if it's the same
crash, or a different one.

There are several bugs/crashes in expand_args_extended():

1) It does not check the range of the positional parameters.  It
  happily parses and uses for example $[999].  I didn't try it,
  but I guess it causes random memory access.

I believe that SkipNTokens just stops when there is no more to consume.
Then $[999] will be empty if there are less then 1000 words in the string.


2) In the "if (is_single_arg)" it uses the token returned by
  PeekToken as the base for further parsing functions.  This is
  strictly forbidden because PeekToken returns a pointer to a
  static buffer.

Not true. With 'is_single_arg' true 'upper' will always be -1, so no other
parse-function will be called on the string.

Then, why do you not write

 if (is_single_arg)
   ...
 *else* if (upper >= 0)
   ...

?


Probably because I didn't think of it as I added is_single_arg later to deal with the fact that $0 should be the same as $[0].

--

Hm, now that I think about it, the new functionality deviates from
the way the old $0 ... worked in important ways which should be
fixed:

* A range of args like $[0-1] is expanded to the original text,
  not the unquoted form.

   I.e. in 'foo "a0"  "a1"  "a2"'

  $[0-1] is expanded to '"a0"  "a1"' but should be expanded to
  'a0 a1'.

Do we really want that? It prevents passing arguments on to another function. The way it is now (quote-persistent) is as $* has worked, and is essensial for pasing multiple parameters on to another function without risking that they split into more parameters than intended. I believe that the current way is a good default until the variable quote system has been implemented.


* Currently, $* is broken too as it does not remove quoting but
  just copies the input string.

See above.


* According to the man page, things like $[*] or $[3-] are
  removed from the command line if there are no arguments.  This
  is wrong as it prevents that the string '$[*]' is passed to
  another command.  These variable should not be treated
  specially.  Currently, not even $[0] is identical to $0.


The old variables also work in this way, so it's only trying to mimic the old behaviour. All my tests make $[0] identical to $0. In what way are they different?

* The code is written too complicated too understand easily (as
  you can see from the fact that I only understood about 50% of
  it), and it's nested too deep.


The deepest nestings are in the actual parsing of the parameters, and that's hard to do without the nesting. Regarding the actual parameter extraction I treid to write useful comments to tell what's going on and why. I'm not sure what more can be done.

/Viktor

Reply via email to