Re: [PATCH] runtest: Allow multi-word arguments
Jacob Bachmeyer <[email protected]> Mon, 17 Apr 2023 21:55:28 -0500
| Newsgroups | gmane.comp.sysutils.dejagnu.general |
|---|---|
| Message-ID | <[email protected]> |
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. > Please send a report of this issue to [email protected] (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)