From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26374 invoked by alias); 20 May 2008 06:38:37 -0000 Received: (qmail 26366 invoked by uid 22791); 20 May 2008 06:38:36 -0000 X-Spam-Check-By: sourceware.org Received: from viper.snap.net.nz (HELO viper.snap.net.nz) (202.37.101.25) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 20 May 2008 06:38:09 +0000 Received: from kahikatea.snap.net.nz (173.63.255.123.dynamic.snap.net.nz [123.255.63.173]) by viper.snap.net.nz (Postfix) with ESMTP id 899083D9A2D; Tue, 20 May 2008 18:38:05 +1200 (NZST) Received: by kahikatea.snap.net.nz (Postfix, from userid 1000) id B049C8FC6D; Tue, 20 May 2008 18:38:02 +1200 (NZST) From: Nick Roberts MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <18482.29129.410026.264704@kahikatea.snap.net.nz> Date: Tue, 20 May 2008 16:10:00 -0000 To: Joel Brobecker Cc: Eli Zaretskii , gdb-patches@sources.redhat.com Subject: Re: [PATCH] New annotation for threads In-Reply-To: <20080520035226.GA4669@adacore.com> References: <18440.29531.916381.569346@kahikatea.snap.net.nz> <20080429023735.GD841@adacore.com> <18454.43094.168458.742737@kahikatea.snap.net.nz> <20080501181758.GD3801@adacore.com> <18458.21177.959458.278174@kahikatea.snap.net.nz> <20080501233703.GF3801@adacore.com> <18458.23326.25887.70597@kahikatea.snap.net.nz> <18478.48682.13900.951343@kahikatea.snap.net.nz> <18479.62120.536436.427524@kahikatea.snap.net.nz> <20080520035226.GA4669@adacore.com> X-Mailer: VM 7.19 under Emacs 22.2.50.2 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: 2008-05/txt/msg00591.txt.bz2 Joel Brobecker writes: > > 2008-05-18 Nick Roberts > > > > * annotate.c (annotate_new_thread): New function for new-thread > > annotation. > > > > * annotate.h: (annotate_new_thread): New extern. > > > > * thread.c (add_thread_with_info): Use it. > > > > * Makefile.in (thread.o): Add dependency on annotate.h. > > (note sure if the empty lines between each entry was intended - just > make sure to remove them when entering this in the ChangeLog). OK. > I'm not terribly happy about this, but that's nonetheless a reasonable > compromise. So this part is OK. > > > 2008-05-18 Nick Roberts > > > > * gdb.base/annota1.exp: Test for new annotation. > > I have a few questions regarding this part: > > > +set testfile "watch_thread_num" > > +set srcfile ${testfile}.c > > +set binfile ${objdir}/${subdir}/${testfile} > > +set gdb_prompt $old_gdb_prompt ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > + > > +if { ![get_compiler_info ${binfile}] && [gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug nowarnings}] == "" } { > > + > > + gdb_exit > > + gdb_start > > + gdb_reinitialize_dir $srcdir/$subdir > > + gdb_load ${binfile} > > + if { ![runto main] } then { > > + fail "run to main" > > + return > > I think this will cause the test to exit before you had a chance > to restore the gdb_prompt. I restore it before this test so I don't think so. > I suggest you put the entire block > of code inside a function, and then call that function at the end > of the script. That way, you can write the body of the function > as if it was its own script (the practice is to "if gdb_compile != "" > then return..." so that you don't need increasing levels of nesting. > but that's very minor, I don't mind the way you wrote it) Presumably it also means more return points. Usually the test is done at the start of the script where nesting would be more inconvenient. > Another alternative would be to have a new specific script. It wouldn't > be any more work at all, and might be cleaner (although, you still do > have to be careful about the gdb_prompt). I'll put it inside a function. I don't want to create a separate script because I think this test belongs with all the other tests for level 2 annotations. > > + send_gdb "set annotate 2\n" > > + gdb_expect { > > + -re "set annotate 2\r\n$gdb_prompt$" {} > > + } > > + > > + send_gdb "next 2\n" > > + gdb_expect { > > + -re ".*\032\032new-thread" { > > + pass "new thread" > > + } > > + timeout { fail "new thread (timeout)" } > > + } > > I think you should be able to use gdb_test in this case, no? > I realize that you are only repeating the practice that is currently > used throughout the file, but we're trying to avoid the send_gdb/ > gdb_expect sequence whenever we can, and use either gdb_test or > gdb_test_multiple. For one thing, both already handle the various > error conditions, including the timeout. Well, I can't get gdb_test to work. It says in annota1.exp: # NOTE: When this prompt is in use the gdb_test procedure cannot be used because # it assumes that the last char after the gdb_prompt is a white space. This is not # true with this annotated prompt. So we must use send_gdb and gdb_expect. # -- Nick http://www.inet.net.nz/~nickrob