Re: Save the length of inserted breakpoints

[Daniel Jacobowitz <drow at false dot org>]

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