Re: [PATCH] runtest: Allow multi-word arguments

[Jacob Bachmeyer]

Christoph Muellner wrote:
> From: Christoph Müllner <christoph.muellner@vrull.eu>
>
> A recent change (5fafcd43) introduced a command line argument cache,
> that ensures that command line arguments have a higher priority
> than configuration files.
>
> That cache uses the following code to save and restore:
>     save_cmd_var:
>       upvar 1 $name target_var
>     restore_cmd_vars"
>       uplevel 1 set $name $value
>
> This works well unless $value becomes a multi-word string
> (i.e. a string that contains spaces), because in this case
> the command becomes:
>     uplevel 1 set $name arg-word0 arg-word1 ...
> Obviously this will trigger the following error:
>     wrong # args: should be "set varName ?newValue?"
> Quoting "$value" does not help, because the quotes are evaluated before
> executing set.
>   

Please send a report of this issue to bug-dejagnu@gnu.org (do *not* CC 
anyone on that email---the GNU debbugs instance will forward it to the 
interested parties and put it in the public archive for anyone else 
interested in this particular bug) to open a bug report for tracking 
this issue and assign a bug number in the GNU debbugs system.  I would 
prefer that the bug records properly credit you for reporting the 
problem.  (The admonition against CC'ing other people on a bug report is 
due to the way those emails are processed; GMail in particular likes to 
eat the message relayed through debbugs that has the actual bug number 
if the original report is also sent directly.)

> Let's fix this by using upvar for the restore code as well:
>     upvar 1 $name target_var
>     set $target_var "$value"
> Here, the quotes will be evaluated when executing the set command.
>   

The contents of the target variable will also be substituted as the name 
of the variable to set, so that will set a strangely-named (local) 
variable instead of restoring the intended value in the calling 
context.  If the testsuite did not catch this, that is also a deficiency 
in the testsuite (which is known to be very incomplete).  The quotes are 
also useless in this case if set is invoked directly; the problem is 
that uplevel does an implicit eval, which is allowing a list to be 
treated as multiple arguments.

I will write a regression test and a proper fix after you get a bug 
number assigned.  I will take of this now that I know that there is a 
problem.  (The correct solution is probably to use list to properly 
quote the set command and its arguments through the implicit eval that 
uplevel performs.)


-- Jacob  (yes, it is me at both email addresses)