From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4419 invoked by alias); 11 Sep 2014 15:57:05 -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 4407 invoked by uid 89); 11 Sep 2014 15:57:04 -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,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pd0-f176.google.com Received: from mail-pd0-f176.google.com (HELO mail-pd0-f176.google.com) (209.85.192.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 11 Sep 2014 15:57:03 +0000 Received: by mail-pd0-f176.google.com with SMTP id y13so9900497pdi.21 for ; Thu, 11 Sep 2014 08:57:01 -0700 (PDT) X-Received: by 10.68.192.35 with SMTP id hd3mr2892834pbc.144.1410451021440; Thu, 11 Sep 2014 08:57:01 -0700 (PDT) Received: from seba.sebabeach.org.gmail.com (173-13-178-50-sfba.hfc.comcastbusiness.net. [173.13.178.50]) by mx.google.com with ESMTPSA id te8sm1520343pab.34.2014.09.11.08.57.00 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 11 Sep 2014 08:57:00 -0700 (PDT) From: Doug Evans To: Simon Marchi Cc: Subject: Re: [PATCH v3] Introduce remote_target_is_gdbserver References: <1410447276-21821-1-git-send-email-simon.marchi@ericsson.com> Date: Thu, 11 Sep 2014 15:57:00 -0000 In-Reply-To: <1410447276-21821-1-git-send-email-simon.marchi@ericsson.com> (Simon Marchi's message of "Thu, 11 Sep 2014 10:54:36 -0400") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2014-09/txt/msg00359.txt.bz2 Simon Marchi writes: > This patch introduces a function in gdbserver-support.exp to find out > whether the current target is GDBserver. > > The code was inspired from gdb.trace/qtor.exp, so it replaces the code > there by a call to the new function. > > New in v3: > - Remove useless "pass" in remote_target_is_gdbserver. > - Coding style in qtro.exp (braces in condition). > - Changelog entry about qtro.exp. > > gdb/testsuite/ChangeLog: > > * gdb.trace/qtro.exp: Replace gdbserver detection code by... > * lib/gdbserver-support.exp (remote_target_is_gdbserver): New > function. > --- > gdb/testsuite/gdb.trace/qtro.exp | 14 +------------- > gdb/testsuite/lib/gdbserver-support.exp | 18 ++++++++++++++++++ > 2 files changed, 19 insertions(+), 13 deletions(-) > > diff --git a/gdb/testsuite/gdb.trace/qtro.exp b/gdb/testsuite/gdb.trace/qtro.exp > index 22b5051..700c157 100644 > --- a/gdb/testsuite/gdb.trace/qtro.exp > +++ b/gdb/testsuite/gdb.trace/qtro.exp > @@ -98,19 +98,7 @@ if { $traceframe_info_supported == -1 } { > } > > # Check whether we're testing with our own GDBserver. > -set is_gdbserver -1 > -set test "probe for GDBserver" > -gdb_test_multiple "monitor help" $test { > - -re "The following monitor commands are supported.*debug-hw-points.*remote-debug.*GDBserver.*$gdb_prompt $" { > - set is_gdbserver 1 > - pass $test > - } > - -re "$gdb_prompt $" { > - set is_gdbserver 0 > - pass $test > - } > -} > -if { $is_gdbserver == -1 } { > +if { ![remote_target_is_gdbserver] } { > return -1 > } > > diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp > index 026a937..423c729 100644 > --- a/gdb/testsuite/lib/gdbserver-support.exp > +++ b/gdb/testsuite/lib/gdbserver-support.exp > @@ -436,3 +436,21 @@ proc mi_gdbserver_start_multi { } { > > return [mi_gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport] > } > + > +# Return true if the current remote target is an instance of gdbserver. > + > +proc remote_target_is_gdbserver { } { > + global gdb_prompt > + > + set is_gdbserver 0 > + set test "Probing for GDBserver" > + > + gdb_test_multiple "monitor help" $test { > + -re "The following monitor commands are supported.*Quit GDBserver.*$gdb_prompt $" { > + set is_gdbserver 1 > + } > + -re "$gdb_prompt $" { > + } > + } > + return $is_gdbserver > +} Hi. The original code allowed for a -1 value of is_gdbserver to handle the case of "can't tell" (e.g. for a timeout or whatever, IIUC). While IWBN to not complicate the API of remote_target_is_gdbserver by requiring the caller to have to handle this, maybe it'd be best if the caller did have to watch for -1 and not just assume "not gdbserver": maybe a different test will want to handle all three cases (can't-tell, no, or yes). E.g., initialize is_gdbserver to -1, and watch for a -1 value before returning. proc remote_target_is_gdbserver { } { global gdb_prompt set is_gdbserver -1 set test "Probing for GDBserver" gdb_test_multiple "monitor help" $test { -re "The following monitor commands are supported.*Quit GDBserver.*$gdb_prompt $" { set is_gdbserver 1 } -re "$gdb_prompt $" { set is_gdbserver 0 } } if { $is_gdbserver == -1 } { verbose -log "can't tell if using gdbserver or not" # or whatever set $is_gdbserver 0 # <<<< this part I'm not sure about } return $is_gdbserver Also, I see an earlier version of the patch first did a check for [is_remote_target] before calling target_is_gdbserver, and the new version of the patch changes that to just calling remote_target_is_gdbserver. Since the function remote_target_is_gdbserver can be used regardless of whether the target is remote, let's remove "remote_" from the name. ie., go back to target_is_gdbserver. Hmmm, another thought. Since this requires an exchange with the target, IWBN to cache the result. There's support for doing this in the harness, grep for gdb_caching_proc.