Re: BUG: improper format string construction in framework.exp

Jacob Bachmeyer <[email protected]> Tue, 30 Oct 2018 18:48:21 -0500
Newsgroups gmane.comp.sysutils.dejagnu.bugs
Message-ID <[email protected]>
Andreas Schwab wrote:
> On Okt 29 2018, Jacob Bachmeyer <[email protected]> wrote:
>
>   
>> Andreas Schwab wrote:
>>     
>>> On Okt 28 2018, Jacob Bachmeyer <[email protected]> wrote:
>>>   
>>>       
>>>> diff --git a/lib/framework.exp b/lib/framework.exp
>>>> index 6cb93c5..50ac757 100644
>>>> --- a/lib/framework.exp
>>>> +++ b/lib/framework.exp
>>>> @@ -800,7 +800,7 @@ proc record_test { type message args } {
>>>>       global multipass_name
>>>>      if { $multipass_name != "" } {
>>>> -       set message [format "$type: %s: $message" "$multipass_name"]
>>>> +       set message [format "%s: %s: %s" "$type" "$multipass_name" "$message"]
>>>>     
>>>>         
>>> What's the point of using format in the first place?
>>>
>>>           set message "$type: $multipass_name: $message"
>>>   
>>>       
>> That code is ancient and was in the initial commit in whatever became the
>> DejaGnu Git repository.  I presume that some version of Tcl does not
>> correctly interpolate variables if the name contains underscore.
>>     
>
> Then "$multipass_name" wouldn't work either.  In fact, tcl does not do
> word splitting, so the quotes around the variables are actually useless.
>   

You are right, and I overlooked that earlier.  However, there are 
currently no regression tests for this code, so I would suggest keeping 
to minimal changes for now.  I will remember your suggestion and will 
probably write a cleanup patch for this after writing MULTIPASS 
regression tests.

On a side note, I have been calling it "MULTIPASS" because that is the 
name of the variable that activates this feature.  What is the proper 
name for this feature as it should appear in the manual?

-- Jacob