Re: Save the length of inserted breakpoints

[Mark Kettenis <mark dot kettenis at xs4all dot nl>]

> Date: Tue, 11 Apr 2006 17:46:13 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Fri, Mar 03, 2006 at 12:01:52AM +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.
> > 
> > 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?
> 
> All true.  Do you like this version better?  I think it's much
> more elegant.  target_insert_breakpoint et al. now take the
> struct bp_location, instead of just the shadow contents cache.
> They can fill in whatever they choose.

I'm very sorry Daniel, but I think this is a bad idea.  Passing down
struct bp_location makes the interface between the low-level tdep code
and the high-level breakpoint code much less clear.  The low-level
code really should not know about the details of the breakpoint
implementation because people will be tempted to abuse it.  And
changing the breakpoint interface will become a pain because suddenly
we will need to change all targets as well.

Mark