From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 83783 invoked by alias); 4 Dec 2018 15:54:24 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 81856 invoked by uid 89); 4 Dec 2018 15:54:23 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=sought, HContent-Transfer-Encoding:8bit X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 04 Dec 2018 15:54:21 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 466A67F41D; Tue, 4 Dec 2018 15:54:20 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6E9AD5DD8A; Tue, 4 Dec 2018 15:54:19 +0000 (UTC) Subject: Re: [PATCH] gdb/testsuite/sim: Remove redundant setting of timeout To: Simon Marchi , Andrew Burgess References: <20181204113345.717-1-andrew.burgess@embecosm.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <07a27e33-ca57-f4be-a055-8d4070b03554@redhat.com> Date: Tue, 04 Dec 2018 15:54:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2018-12/txt/msg00034.txt.bz2 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 ^^^^^^^^^^^^^^^^^^^^^^^ ... Thanks, Pedro Alves