From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26301 invoked by alias); 2 May 2011 15:30:20 -0000 Received: (qmail 26278 invoked by uid 22791); 2 May 2011 15:30:19 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 02 May 2011 15:30:04 +0000 Received: (qmail 14092 invoked from network); 2 May 2011 15:30:04 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 2 May 2011 15:30:04 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649) Date: Mon, 02 May 2011 15:30:00 -0000 User-Agent: KMail/1.13.5 (Linux/2.6.35-28-generic; KDE/4.6.2; x86_64; ; ) Cc: Jan Kratochvil , Marek Polacek , Joel Brobecker References: <4DB82F26.30801@redhat.com> <201105021519.25614.pedro@codesourcery.com> <20110502145229.GA22957@host1.jankratochvil.net> In-Reply-To: <20110502145229.GA22957@host1.jankratochvil.net> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201105021630.04082.pedro@codesourcery.com> X-IsSubscribed: yes 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 X-SW-Source: 2011-05/txt/msg00018.txt.bz2 On Monday 02 May 2011 15:52:29, Jan Kratochvil wrote: > On Mon, 02 May 2011 16:19:25 +0200, Pedro Alves wrote: > > On Sunday 01 May 2011 10:16:30, Jan Kratochvil wrote: > > > The "complete" command appraoch does introduce this new kind of race. > > > > > > But the patch can be commited in two parts if it is preferred although > > > reviewing these racy send_gdb-gdb_expect cases for the intermediate step is > > > tricky and it gets dropped immediately afterwards. > > > > What do you mean is dropped immediately afterwards? > > Replacing this send_gdb + gdb_expect by gdb_test "complete ..." makes the GDB > codebase/testsuite more maintainable so I thought it could be changed now. > > I found easier to replace the current constructs by gdb_test "complete ..." at > once although one can fix the gdb_expect first and delete it afterwards if you > wish. > > Or are you against the replacement by gdb_test "complete ..."? The "\t" method of completion interacts with readline, the "complete command" method doesn't. I think it's useful and important to test the "\t" version, especially since it's what CLI users are using. As principle, I don't like rewriting to make a not understood problem go away. In this particular case, since it would be desirable to keep at least one instance of the original form, we get to fix at least that instance. But when we know how to fix it, we can fix all of the instances. And then the original motivation to rewrite using a different method disappears or at least diminishes. > > I have checked the code paths (despite what Tom says) and personally I cannot > imagine a difference between \t and the "complete" command. > > > > > > @@ -410,7 +365,7 @@ gdb_expect { > > > > timeout {fail "(timeout) complete 'p \"break1.'"} > > > > } > > > > } > > > > - -re "^p \"break1\\..*$" > > > > + -re "^p \"break1\\...*$" > > > > { send_gdb "\n" > > > > gdb_expect { > > > > -re ".*$gdb_prompt $" { fail "complete 'p \"break1.'"} > > > > > > I do not see this change as valid/relevant. > > > > The pattern above reads: > > > > -re "^p \"break1\\.c\"$"\ > > ... > > -re "^p \"break1\\..*$" > > ... > > > > It looked like "^p \"break1\\.c" could wrongly match the latter pattern, > > if the "c" wasn't in the buffer yet? > > Aha. But this testcase always FAILs (which it always considers as XFAIL) > because: > (1) gdb.base/break1.o prevents the completion (during in-tree build) > (2) GDB 7.3.50.20110502-cvs now completes it (with bundled readline) as: > >p "break1.c < (note the trailing space) > instead of expected > >p "break1.c"< > so it FAILs/XFAILs anyway. > So I am not sure what should be there when it cannot work anyway. Thanks. Sounds like break1.c was added (2003) after the test was originally written (199[89], and since the completion never worked as intended by the author of test, always XFAILing, it was never noticed break1.o was in the way. I somehow not noticed the \", making it so that my change wasn't enough anyway (needed yet one more dot, or something better). I'll remove it. -- Pedro Alves