From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12137 invoked by alias); 8 Nov 2010 18:43:50 -0000 Received: (qmail 12129 invoked by uid 22791); 8 Nov 2010 18:43:49 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 08 Nov 2010 18:43:42 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 3B4FC2BAC90; Mon, 8 Nov 2010 13:43:41 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id fCqM7NUKX+dR; Mon, 8 Nov 2010 13:43:41 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id EAAD92BAC87; Mon, 8 Nov 2010 13:43:40 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 49722145907; Mon, 8 Nov 2010 10:43:35 -0800 (PST) Date: Mon, 08 Nov 2010 18:43:00 -0000 From: Joel Brobecker To: Thiago Jung Bauermann Cc: gdb-patches@sourceware.org Subject: Re: [patch 1/2] Convert hardware watchpoints to use breakpoint_ops Message-ID: <20101108184335.GG2933@adacore.com> References: <1282074071.2606.702.camel@hactar> <201010161843.43062.pedro@codesourcery.com> <1287534691.2686.17.camel@hactar> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1287534691.2686.17.camel@hactar> User-Agent: Mutt/1.5.20 (2009-06-14) 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/msg00127.txt.bz2 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. 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: 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) [...] -- Joel