From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <SRS0=yNqg=5B=lancelotsix.com=lsix@sourceware.org>
Received: from lndn.lancelotsix.com (vps-42846194.vps.ovh.net [IPv6:2001:41d0:801:2000::2400])
	by sourceware.org (Postfix) with ESMTPS id EAD203858D35
	for <gdb-patches@sourceware.org>; Wed,  4 Jan 2023 17:41:22 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EAD203858D35
Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=lancelotsix.com
Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=lancelotsix.com
Received: from ubuntu.lan (cust120-dsl54.idnet.net [212.69.54.120])
	by lndn.lancelotsix.com (Postfix) with ESMTPSA id 315C18018A;
	Wed,  4 Jan 2023 17:41:22 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=lancelotsix.com;
	s=2021; t=1672854082;
	bh=M4AiHSwfqfuuTr9Jz3jdVCcpCVweRgsvmONUYWca+mY=;
	h=Date:From:To:Cc:Subject:References:In-Reply-To:From;
	b=tPhZ45gVLhYBmsqIn7jmwHjqZxSrGUQEVsErovxQBDeMRdQKFPEsrnnhp2MDtYSF3
	 Z4H/ZL2qy7z1Cg/7i7E/ipivkJgRfG8l5Y770XCOGYJP3eyi41GA9uqd4p+RUymlaF
	 +FXC/Kq6kMoQUutxDCz/cNER7TaTHqq924UJxJ9l5irEQuHIbMvT3R2egOslS/yAtv
	 i6JE8sRToqNnW9Q32tRzYWBQfRKi2pxZJSh72bnLIDr3S711AG6bm1hccjyT5Y7v3y
	 9g/OTvIYKU49jQ59iBeGQp7LFd/RH7AtzoPXV4CMubnB6ZWVVTurN2J3HpaiZtUOKx
	 d6ovq6aqseipA==
Date: Wed, 4 Jan 2023 17:40:38 +0000
From: Lancelot SIX <lsix@lancelotsix.com>
To: Simon Marchi <simon.marchi@polymtl.ca>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] gdb: use gdb_test_multiple in gdb_breakpoint
Message-ID: <20230104174038.aeau2an7tmufa3pj@ubuntu.lan>
References: <20230103192216.108444-1-simon.marchi@polymtl.ca>
 <20230104091559.rqo2774ba566j5sg@ubuntu.lan>
 <9b050608-4bb6-dbc3-69f6-ebc9ad2cc303@polymtl.ca>
 <20230104161817.qbvbspsqcqrjw6w2@ubuntu.lan>
 <7d38d82e-1beb-c3dc-441a-b4e455351020@polymtl.ca>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <7d38d82e-1beb-c3dc-441a-b4e455351020@polymtl.ca>
X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (lndn.lancelotsix.com [0.0.0.0]); Wed, 04 Jan 2023 17:41:22 +0000 (UTC)
X-Spam-Status: No, score=-10.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org
List-Id: <gdb-patches.sourceware.org>

On Wed, Jan 04, 2023 at 11:22:55AM -0500, Simon Marchi via Gdb-patches wrote:
> 
> 
> On 1/4/23 11:18, Lancelot SIX wrote:
> >> Oh, right, I missed that.  I folded the test name in the
> >> gdb_test_multiple call (removed the test_name var) and used
> >> $gdb_test_name in the fail call.  I updated the patch locally.  Should I
> >> add your Reviewed-By after those changes?
> >>
> >> Simon
> >>
> > 
> > Yes, with those change, feel free to add
> > 
> > Reviewed-By: Lancelot Six <lancelot.six@amd.com>
> > 
> > Best,
> > Lancelot.
> 
> Thanks, pushed!
> 
> Simon

Hi,

I just saw the resulting patch, and is seems that $test_name is
(conditionally) used after the gdb_test_multiple:

    diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
    index 135ace68d5e..ba16b2ab315 100644
    --- a/gdb/testsuite/lib/gdb.exp
    +++ b/gdb/testsuite/lib/gdb.exp
    @@ -639,18 +639,15 @@ proc gdb_breakpoint { linespec args } {
            set print_pass 1
         }
    
    -    set test_name "gdb_breakpoint: set breakpoint at $linespec"
    -
    -    send_gdb "$break_command $linespec\n"
         # The first two regexps are what we get with -g, the third is without -g.
    -    gdb_expect 30 {
    +    gdb_test_multiple "$break_command $linespec" "gdb_breakpoint: set breakpoint at $linespec" {
            -re "$break_message \[0-9\]* at .*: file .*, line $decimal.\r\n$gdb_prompt $" {}
            -re "$break_message \[0-9\]*: file .*, line $decimal.\r\n$gdb_prompt $" {}
            -re "$break_message \[0-9\]* at .*$gdb_prompt $" {}
            -re "$break_message \[0-9\]* \\(.*\\) pending.*$gdb_prompt $" {
                    if {$pending_response == "n"} {
                            if { $print_fail } {
    -                               fail $test_name
    +                               fail $gdb_name_name
                            }
                            return 0
                    }
    @@ -659,35 +656,6 @@ proc gdb_breakpoint { linespec args } {
                    send_gdb "$pending_response\n"
                    exp_continue
            }
    -       -re "A problem internal to GDB has been detected" {
    -               if { $print_fail } {
    -                   fail "$test_name (GDB internal error)"
    -               }
    -               gdb_internal_error_resync
    -               return 0
    -       }
    -       -re "$gdb_prompt $" {
    -               if { $print_fail } {
    -                       fail $test_name
    -               }
    -               return 0
    -       }
    -       eof {
    -               perror "GDB process no longer exists"
    -               global gdb_spawn_id
    -               set wait_status [wait -i $gdb_spawn_id]
    -               verbose -log "GDB process exited with wait status $wait_status"
    -               if { $print_fail } {
    -                       fail "$test_name (eof)"
    -               }
    -               return 0
    -       }
    -       timeout {
    -               if { $print_fail } {
    -                       fail "$test_name (timeout)"
    -               }
    -               return 0
    -       }
         }
         if { $print_pass } {
            pass $test_name

Just here   ^ $test_name is used, but the patch does not set it anymore.
Looks like that it should become something like:

    set test_name "gdb_breakpoint: set breakpoint at $linespec"
    gdb_test_multiple "$break_command $linespec" $test_name {
      # unchanged
    }
    if { $print_pass } {
      pass $test_name
    }

Sorry I missed that earlier.

Lancelot.