From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 33183 invoked by alias); 4 Dec 2018 16:11:58 -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 33162 invoked by uid 89); 4 Dec 2018 16:11:57 -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= 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 16:11:55 +0000 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8C7F7811C0; Tue, 4 Dec 2018 16:11:54 +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 7F3381054FDC; Tue, 4 Dec 2018 16:11:53 +0000 (UTC) Subject: Re: [PATCH] gdb/testsuite/sim: Remove redundant setting of timeout To: Simon Marchi References: <20181204113345.717-1-andrew.burgess@embecosm.com> <07a27e33-ca57-f4be-a055-8d4070b03554@redhat.com> <37e9f834bac94720a257148d45bd0325@polymtl.ca> Cc: Andrew Burgess , gdb-patches@sourceware.org From: Pedro Alves Message-ID: Date: Tue, 04 Dec 2018 16:11: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: <37e9f834bac94720a257148d45bd0325@polymtl.ca> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2018-12/txt/msg00036.txt.bz2 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