On Mon, Sep 22, 2014 at 07:52:41PM +0100, Thomas Adam wrote: > From: Thomas Adam <tho...@fvwm.org> > > Dominik, > > In taking a look at the new parser code, I decided to run it over > fvwm-themes and it crashed mvwm: > > #0 0x00000b5d519aa865 in expand_vars (input=0x7f7fffff89b0 "+ I Mouse $0 $2 > $3 $4 \"$5\" \"$6\" \"$7\" \"$8\" \"$9\"", pc=0x7f7fffff87c0, addto=1, > ismod=0, > cond_rc=0x7f7fffff88d8, exc=0xb6027da2e00) at expand.c:1267 > > This is easy to reproduce with the following snippet (I just typed it into > MvwmConsole): > > DestroyFunc CrashMe > AddtoFunc CrashMe > + I Mouse $0 $2 $3 $4 \"$5\" \"$6\" \"$7\" \"$8\" \"$9\" > > CrashMe
> The reason for this is the argument list includes the command itself, and > since we also have all nine positonal arguments, in checking pos->args[n] in > expand_vars() we overflow the pos->args[] array. :-p I knew that this was going to crash. The old code was just awfull and extremely difficult to get right again. > The valid argument size is 11. In changing CMDPARSER_NUM_POS_ARGS to > reflect this, I've also noticed that functions.c also hard-codes 11 as the > default, so I've replaced those occurances with CMDPARSER_NUM_POS_ARGS > instead. Okay, but I think we should leave it at ten and rather use CMDPARSER_NUM_POS_ARGS + 1 instead of 11. When that is fixed, it would improve maintainability to remove the whole line string from the pos args array and give it a separate variable. (I tried it, but don't remember why it was difficult to finish.) Can you commit the modified change, please? Ciao Dominik ^_^ ^_^ -- Dominik Vogt
