This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] gdb/testsuite/sim: Remove redundant setting of timeout
On 12/04/2018 04:08 PM, Simon Marchi wrote:
> On 2018-12-04 10:54, Pedro Alves wrote:
>> On 12/04/2018 03:43 PM, Simon Marchi wrote:
>>> On 2018-12-04 06:33, Andrew Burgess wrote:
>>>> In the config/sim.exp file two functions are defined. Both of these
>>>> functions define local timeout variables and then call gdb_expect,
>>>> which (through a call to get_largest_timeout) will find the local
>>>> definition of timeout.
>>>>
>>>> However, both of these functions set the local timeout to some
>>>> arbitrary value and print a log message for this "new" timeout just
>>>> before returning.
>>>>
>>>> As in both cases, the timeout is a local variable, this final setting
>>>> of the timeout has no effect and can be removed.
>>>
>>> Hi Andrew,
>>>
>>> Can you verify whether the remaining "set timeout" in those functions have any effect at all? As you said, they are just local variables, so I don't expect them to influence the behavior of gdb_expect. Either we need "global timeout", or we pass the timeout directly as an argument to gdb_expect (the latter sounds better).
>>
>> Keep this in mind, from man expect:
>>
>> Expect takes a rather liberal view of scoping. In particular,
>> variables read by commands specific to the Expect program will be sought
>> first from the local scope, and if not found, in the global scope. For
>> example, this obviates the need to place "global timeout" in
>> every procedure
>> you write that uses expect. On the other hand, variables
>> written are always
>> in the local scope (unless a "global" command has been issued). The most
>> common problem this causes is when spawn is executed in a
>> procedure. Outside
>> the procedure, spawn_id no longer exists, so the spawned
>> process is no longer
>> accessible simply because of scoping. Add a "global spawn_id"
>> to such a procedure.
>>
>>
>> Mimicking that behavior, gdb_test, gdb_test_multiple and gdb_expect pick the
>> local timeout variable in the caller via upvar. E.g.:
>>
>> proc gdb_test { args } {
>> global gdb_prompt
>> upvar timeout timeout
>>
>> gdb_expect is a little more disguised, but it does the same, here,
>> in the get_largest_timeout path:
>>
>> proc gdb_expect { args } {
>> ...
>> # A timeout argument takes precedence, otherwise of all the timeouts
>> # select the largest.
>> if [info exists atimeout] {
>> set tmt $atimeout
>> } else {
>> set tmt [get_largest_timeout]
>> }
>> ...
>> }
>>
>> and then get_largest_timeout does:
>>
>> proc get_largest_timeout {} {
>> upvar #0 timeout gtimeout
>> upvar 2 timeout timeout
>> ^^^^^^^^^^^^^^^^^^^^^^^
>> ...
>
> That's very confusing, to say the least.
Don't shoot the messenger. :-)
Thanks,
Pedro Alves