From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18246 invoked by alias); 8 Nov 2010 21:39:02 -0000 Received: (qmail 18233 invoked by uid 22791); 8 Nov 2010 21:38:59 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from e24smtp04.br.ibm.com (HELO e24smtp04.br.ibm.com) (32.104.18.25) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 08 Nov 2010 21:38:53 +0000 Received: from mailhub1.br.ibm.com (mailhub1.br.ibm.com [9.18.232.109]) by e24smtp04.br.ibm.com (8.14.4/8.13.1) with ESMTP id oA8LW0HP021831 for ; Mon, 8 Nov 2010 19:32:00 -0200 Received: from d24av04.br.ibm.com (d24av04.br.ibm.com [9.8.31.97]) by mailhub1.br.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id oA8LjSwc987298 for ; Mon, 8 Nov 2010 19:45:28 -0200 Received: from d24av04.br.ibm.com (loopback [127.0.0.1]) by d24av04.br.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id oA8LclKu023833 for ; Mon, 8 Nov 2010 19:38:47 -0200 Received: from [9.8.6.184] ([9.8.6.184]) by d24av04.br.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id oA8Lcllt023822; Mon, 8 Nov 2010 19:38:47 -0200 Subject: Re: [patch 1/2] Convert hardware watchpoints to use breakpoint_ops From: Thiago Jung Bauermann To: Joel Brobecker Cc: gdb-patches@sourceware.org In-Reply-To: <20101108184335.GG2933@adacore.com> References: <1282074071.2606.702.camel@hactar> <201010161843.43062.pedro@codesourcery.com> <1287534691.2686.17.camel@hactar> <20101108184335.GG2933@adacore.com> Content-Type: text/plain; charset="UTF-8" Date: Mon, 08 Nov 2010 21:39:00 -0000 Message-ID: <1289252326.11397.11.camel@hactar> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit 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: 2010-11/txt/msg00130.txt.bz2 On Mon, 2010-11-08 at 10:43 -0800, Joel Brobecker wrote: > Just for the record, I've started looking at both patches, and already > spent a good hour on them. But I think that I should give myself some > time to to let every piece sink in, before I make an official review. Thank you very much! I'll be glad to clarify any question that may come up during your review of the patch. No need to think too hard about what the patch does in some obscure part, I'll be glad to explain. :-) > I urge another one of us to look at this change, because it's not > completely obvious. In particular, I think it was my suggestion > that prompted the move to using breakpoint_ops for watchpoints. > But it's not obvious that there is a gain. I think the gain shows up > in the second patch, where we avoid having: Yes, I think the 2nd patch became cleaner when I modified it to use breakpoint_ops. And yes, it was your idea. :-) IMHO the code would be even cleaner if I/we ported the existing breakpoints and watchpoints code completely to breakpoint_ops, eliminating functions like print_it_typical. It would make this part of GDB more object-oriented. Though it would perhaps increase the total line count instead of decreasing it, so it's not a clear win. > 1. To define new kinds for range and mask watchpoints > > 2. To program dispatching manually, Eg: > > if (bp->print_one) > bp->print_one (...) > else if (bp->type == hw_watchpoint) > print_one_normal_watchpoint (...) > else if (bp->type == range_watchpoint) > print_one_range_watchpoint (...) > else if (bp->type == mask_watchpoint) > [...] It also avoids adding special cases just for masked and ranged watchpoints. Instead, I added a method to breakpoints_op and if it's defined, I just call it. For instance, in update_watchpoint: + if (b->ops && b->ops->extra_resources_needed) + mem_cnt += b->ops->extra_resources_needed (b); -- []'s Thiago Jung Bauermann IBM Linux Technology Center