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

Christoph Muellner wrote:
> From: Christoph Müllner <[email protected]>
>
> 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.
>   

I believe that a bit more explanation is needed, because this 
description evidences a fundamental misunderstanding of Tcl's evaluation 
rules.  The command actually becomes:
    uplevel 1 {set} {NAME} {ARG-WORD0 ARG-WORD1 ...}
... because Tcl, unlike the shell, does *not* perform word-splitting 
after substituting variables.  The problem is that uplevel performs an 
implicit eval on its arguments, which has the effect of stripping one 
layer of grouping, so the command actually executed in the caller's 
context becomes:
    set NAME ARG-WORD0 ARG-WORD1 ...
... which, as you correctly note, fails with a Tcl error.  The solution 
is to wrap the command passed to uplevel in one extra layer of grouping 
using the Tcl list command, as documented in the eval(n) manpage:  "Note 
that the list command quotes sequences of words in such a way that they 
are not further expanded by the eval command."  Now the command becomes:
    uplevel 1 {{set} {NAME} {ARG-WORD0 ARG-WORD1 ...}}
... which, when evaluated in the caller's context becomes:
    set NAME {ARG-WORD0 ARG-WORD1 ...}
... which is correct and restores the list value for variable NAME.


This issue has been assigned bug number 62982 and is fixed in commit 
c298959ef991b389b64a825f70094738c6a48780, which is now available in Git 
master on Savannah and will be included in the next release.  The 
DejaGnu testsuite has been improved to now detect both the issue itself 
and the error introduced in the original patch offered.


-- Jacob