From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3238 invoked by alias); 2 Mar 2006 23:23:17 -0000 Received: (qmail 3230 invoked by uid 22791); 2 Mar 2006 23:23:17 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Thu, 02 Mar 2006 23:23:15 +0000 Received: from drow by nevyn.them.org with local (Exim 4.54) id 1FEx8b-00060O-Bf; Thu, 02 Mar 2006 18:23:13 -0500 Date: Fri, 03 Mar 2006 01:21:00 -0000 From: Daniel Jacobowitz To: Mark Kettenis Cc: gdb-patches@sourceware.org Subject: Re: Save the length of inserted breakpoints Message-ID: <20060302232313.GA23003@nevyn.them.org> Mail-Followup-To: Mark Kettenis , gdb-patches@sourceware.org References: <20060302221711.GB18830@nevyn.them.org> <200603022301.k22N1qEt008208@elgar.sibelius.xs4all.nl> <20060302231042.GA22458@nevyn.them.org> <200603022318.k22NIiSe027004@elgar.sibelius.xs4all.nl> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200603022318.k22NIiSe027004@elgar.sibelius.xs4all.nl> User-Agent: Mutt/1.5.8i X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-03/txt/msg00066.txt.bz2 On Fri, Mar 03, 2006 at 12:18:44AM +0100, Mark Kettenis wrote: > > > Yuck! It really is ugly. For one thing, I think it is a bit > > > pointless, to add a the BREAKPOINT_FROM_PC() to targets where we know > > > the length of a breakpoint instruction is fixed. Several of the touched targets either have variable length breakpoints, or conceivably could in an architecture revision (e.g. I'm pretty sure there's such an extension for PPC on paper, don't know if it's deployed anywhere). I felt it better to be consistent. > > > Another thing is that I think the order of the arguments of > > > target_remove_breakpoint() is wrong. I think it makes sense to see > > > your "len" argument as the length of the saved memory. Then it is > > > more logical to make "len" the last argument of > > > target_remove_breakpoint(). > > > > > > However, doesn't it make more sense to have target_insert_breakpoint() > > > save the length instead of using BREAKPOINT_FROM_PC() to ask for it? > > > > If you want me to do that, I'll do that instead. It requires touching > > twice as many target functions. Writing the changelog for this one > > took long enough, so forgive me if I wait a while before trying it > > again :-) > > You're touching a fairly fundamental piece of the breakpoint > infrastructure here. I think it is worth thinking about this for a > bit longer. My comments certainly weren't "demands", so I'm perfectly > fine with discussing this a bit more before you rush towards changing > your patch ;-). Nah, I agree that making it symmetric would be good. The problem is that this entire area is riddled with hacks, duplicate ways to accomplish the same goal, and other forms of quirk; I didn't want to get too far into cleaning it up, especially since a third of the files I touched are for targets that probably ought to be deleted anyway. -- Daniel Jacobowitz CodeSourcery