From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15068 invoked by alias); 14 Apr 2015 17:05:14 -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 15055 invoked by uid 89); 14 Apr 2015 17:05:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 14 Apr 2015 17:05:13 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id DDA768E67C; Tue, 14 Apr 2015 17:05:11 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3EH5ApD002217; Tue, 14 Apr 2015 13:05:10 -0400 Message-ID: <552D48C5.2050002@redhat.com> Date: Tue, 14 Apr 2015 17:05:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: Re: [PATCH] Increase timeout in watch-bitfields.exp for software watchpoint References: <1429023644-13403-1-git-send-email-qiyaoltc@gmail.com> <552D31E4.1080503@redhat.com> <86a8yau0qb.fsf@gmail.com> In-Reply-To: <86a8yau0qb.fsf@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-04/txt/msg00536.txt.bz2 On 04/14/2015 05:35 PM, Yao Qi wrote: >>> >>> +# Run tests in BODY with timeout increased by factor of FACTOR. When >>> +# BODY is finished, restore timeout. >>> + >>> +proc with_timeout_factor { factor body } { >>> + global timeout >>> + >>> + set savedtimeout $timeout >>> + if { [target_info exists gdb,timeout] >>> + && $timeout < [target_info gdb,timeout] } { >>> + set oldtimeout [target_info gdb,timeout] >>> + } else { >>> + set oldtimeout $timeout >>> + } >>> + set timeout [expr $oldtimeout * $factor] >> >> The "timeout" variable is special. gdb_test/gdb_test_multiple/expect >> will take into account a local "timeout" variable in the callers >> scope too, not just the global. So this should be taking that >> into account as well. The old code didn't need to do that because it >> was code at the global scope. See the upvars in gdb_expect. I think >> we should do the same here. We should probably move >> that "get me highest timeout" bit of code to a shared >> procedure (adjusted to "upvar 2 timeout timeout", most likely). > > I don't think I fully understand you... Why do we need such shared proc > to get timeout? Isn't simpler to just use "upvar timeout timeout" at > the beginning of with_timeout_factor? like this: > > proc with_timeout_factor { factor body } { > upvar timeout timeout > > and in watch-bitfields.exp proc test_watch_location and > test_regular_watch, use "global timeout"? With a global timeout of 60, this: proc foo {} { set timeout 1 gdb_test ... } still runs gdb_test with a timeout of 60, because, 60 > 1. so, what should this mean: proc foo {} { set timeout 1 gdb_test ... with_timeout_factor 10 { gdb_test ... } } is the timeout within the with block max(1, 60 * 10), or max(1 * 10, 60) ? And this: proc foo {} { set timeout 100 gdb_test ... with_timeout_factor 10 { gdb_test ... } } is it max(100, 60 * 10), or max(100 * 10, 60) ? It seems to me that we should define what this does, and document it. gdb_test etc. always take the max of local, global and board timeouts. In your current patch, the factor is applied after selecting the max of global and board timeouts. So I think that it should be simplest to say that the the factor is applied after determining the maximum between local, global and board timeouts. So the shared proc would look like this: # Of all the timeouts select the largest. Uses the local "timeout" # variable of the scope two levels above. proc get_largest_timeout {} { upvar #0 timeout gtimeout upvar 2 timeout timeout set tmt 0 if [info exists timeout] { set tmt $timeout } if { [info exists gtimeout] && $gtimeout > $tmt } { set tmt $gtimeout } if { [target_info exists gdb,timeout] && [target_info gdb,timeout] > $tmt } { set tmt [target_info gdb,timeout] } if { $tmt == 0 } { # Eeeeew. set tmt 60 } return $tmt } Used like: proc with_timeout_factor { factor body } { global timeout set savedtimeout $timeout set timeout [get_largest_timeout] set code [catch {uplevel 1 $body} result] if {$code == 1} { global errorInfo errorCode return -code $code -errorinfo $errorInfo -errorcode $errorCode $result } else { return -code $code $result } set timeout $savedtimeout } and: proc gdb_expect { args } { if { [llength $args] == 2 && [lindex $args 0] != "-re" } { set atimeout [lindex $args 0] set expcode [list [lindex $args 1]] } else { set expcode $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 you don't need those "global" in test_watch_location / test_regular_watch either. Thanks, Pedro Alves