From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16741 invoked by alias); 10 Nov 2011 15:51:38 -0000 Received: (qmail 16732 invoked by uid 22791); 10 Nov 2011 15:51:37 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 10 Nov 2011 15:51:24 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=EU1-MAIL.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1ROWuR-0004Ie-R9 from pedro_alves@mentor.com for gdb-patches@sourceware.org; Thu, 10 Nov 2011 07:51:24 -0800 Received: from scottsdale.localnet ([172.16.63.104]) by EU1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.1830); Thu, 10 Nov 2011 15:51:22 +0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [patch 4/8] Download tracepoint locations and track its status Date: Thu, 10 Nov 2011 15:51:00 -0000 User-Agent: KMail/1.13.6 (Linux/2.6.38-12-generic; KDE/4.7.2; x86_64; ; ) Cc: Yao Qi References: <4EB8C551.9090609@codesourcery.com> <4EB8CC79.4010100@codesourcery.com> <4EB9F7BF.7000005@codesourcery.com> In-Reply-To: <4EB9F7BF.7000005@codesourcery.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201111101551.20428.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-11/txt/msg00275.txt.bz2 On Tuesday 08 November 2011 06:30:17, Yao Qi wrote: > @@ -10660,6 +10713,8 @@ update_global_location_list (int should_insert) > || (gdbarch_has_global_breakpoints (target_gdbarch)))) > insert_breakpoint_locations (); > > + download_tracepoint_locations (); Should be guarded by `should_insert'. > - /* Skip LOCP entries which will definitely never be needed. > + /* skip LOCP entries which will definitely never be needed. Drop this spurious change please. > @@ -10491,7 +10539,9 @@ update_global_location_list (int should_insert) > if it should be inserted in case it will be > unduplicated. */ > if (loc2 != old_loc > - && unduplicated_should_be_inserted (loc2)) > + && unduplicated_should_be_inserted (loc2) > + && (is_tracepoint (old_loc->owner) > + == is_tracepoint (loc2->owner))) What if both are tracepoints, but of different kind? I think this might be better handled within breakpoint_locations_match (called just above). > /* Nonzero if this is not the first breakpoint in the list > - for the given address. */ > + for the given address. In current implementation, location > + of tracepoint never be duplicated with other locations of > + tracepoints and other kinds of breakpoints, because two locations > + at the same address may have different actions, so both of > + these locations should be downloaded. and so that "tfind N" always works. Please drop the "In current implementation" bit, and the "it is possible" comment. > + /* Locations of tracepoints never be duplicated. */ A verb is missing. "can never be", perhaps? > + if (is_tracepoint (left->owner)) > + gdb_assert (left->duplicate == 0); `duplicate' is a boolean. Make that `gdb_assert (!left->duplicate)'. > + if (is_tracepoint (right->owner)) > + gdb_assert (right->duplicate == 0); > + gdb_assert (loc->inserted == 0); Boolean ==> `gdb_assert (!loc->inserted)' -- Pedro Alves