From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4921 invoked by alias); 13 Apr 2006 22:59:27 -0000 Received: (qmail 4910 invoked by uid 22791); 13 Apr 2006 22:59:24 -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, 13 Apr 2006 22:59:22 +0000 Received: from drow by nevyn.them.org with local (Exim 4.54) id 1FUAmT-00085e-B9; Thu, 13 Apr 2006 18:59:17 -0400 Date: Thu, 13 Apr 2006 22:59:00 -0000 From: Daniel Jacobowitz To: Mark Kettenis Cc: eliz@gnu.org, gdb-patches@sourceware.org Subject: Re: Save the length of inserted breakpoints Message-ID: <20060413225917.GA30759@nevyn.them.org> Mail-Followup-To: Mark Kettenis , eliz@gnu.org, gdb-patches@sourceware.org References: <20060302221711.GB18830@nevyn.them.org> <200603022301.k22N1qEt008208@elgar.sibelius.xs4all.nl> <20060411214613.GA702@nevyn.them.org> <200604120943.k3C9hYJ8012016@elgar.sibelius.xs4all.nl> <20060412125712.GA22145@nevyn.them.org> <200604121837.k3CIbMwu004466@elgar.sibelius.xs4all.nl> <20060412184717.GA29980@nevyn.them.org> <200604132213.k3DMDeBX026776@elgar.sibelius.xs4all.nl> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200604132213.k3DMDeBX026776@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-04/txt/msg00175.txt.bz2 On Fri, Apr 14, 2006 at 12:13:40AM +0200, Mark Kettenis wrote: > > > I can think of plenty of other places where another constant might > > > be useful. You might want to record which hardware breakpoint > > > registers were used, for instance, instead of digging around > > > to figure out which ones to clear. Adding a new member to > > > "struct bp_target" for that would be easy. > > But we're talking specifically about the interface for software > breakpoints here aren't we? Or are we redesigning the target > breakpoint interface here? If we are, I think we should try to come > up with a design of some sort before rushing to implement it. I'm sure you realize that we already pass the shadow contents buffer to the hardware breakpoint insertion routines. I simply updated that. See below for my comments on redesigning interfaces. > > FWIW, I agree with Daniel: it is better to pass a struct than its > > individual members, especially if we expect different targets to use > > different members of that struct. In other words, passing a struct > > eases future maintenance pains. > > And it obfuscates the interface. Unnecessary layers of abstraction > make software difficult to understand and therefore difficult to > maintain. So unless someone can make a reasonable case why we need a > more general interface, I'm against it. I think it is far clearer to pass a "bp_target_info" structure to the "target insert a breakpoint" and "target remove a breakpoint" routines than a length. Why the heck do they need a length? Many of them don't even look at it. But it's a target breakpoint and so passing along some target breakpoint information makes perfect sense. It'll use whatever it needs. I'm way tired of discussing this patch, however. I'd like to remind the audience that I'm not trying to clean up the target breakpoint interface; I was trying to fix a nasty debugging problem for ARM where it's perfectly obvious to the user what mode some code is in, but GDB gets lost. My first version was too limited to the problem case and was dismissed as hopelessly ugly. So I cleaned it up, added some abstraction, and now it's "obfuscated". Every time I have to overhaul an interface to fix a bug, I get a little bit more frustrated. The bug took an hour, and this has taken days. By the way, as a general note on my frustration level right now: if instead of this patch, I had posted a clear proposal for an overhauled target breakpoint interface (something I have no interest in doing, if you're wondering), my recent experiences suggest that no one would have been interested in discussing the proposal anyway. I do not think the abstraction is at all unnecessary, or I wouldn't have added it. I presented three reasons why it could be useful: one practical concern with the address modifications performed by the existing BREAKPOINT_FROM_PC interface, one potential use case for hardware breakpoints, and the considerable simplification of future changes to these specific functions. Here's another one. Many monitors have their own table of breakpoints. Perhaps we need to save a breakpoint index returned by the monitor in order to remove the breakpoint. I wouldn't be ashamed to write a monitor that worked that way; creating the breakpoint gives you a handle, to remove the breakpoint you need the handle. This would be a fine place to record the handle. Would you still prefer I just pass address and length? -- Daniel Jacobowitz CodeSourcery