Re: Save the length of inserted breakpoints

[Daniel Jacobowitz <drow at false dot org>]

On Fri, Mar 03, 2006 at 12:01:52AM +0100, Mark Kettenis wrote:
> > Date: Thu, 2 Mar 2006 17:17:11 -0500
> > From: Daniel Jacobowitz <drow@false.org>
> > 
> > This nasty, mechanical patch adds "len" arguments to
> > target_remove_breakpoint and target_remove_hw_breakpoint.  The goal is
> > to allow BREAKPOINT_FROM_PC to include heuristics, which may possibly
> > change between when a breakpoint is inserted and when it is removed;
> > in order to stay in sync, we need to always remove breakpoints in the
> > same way that we inserted them.
> > 
> > There's not much more to say about this patch.  It's big, obvious, and
> > pretty ugly.  Any comments on this?  Does it look OK?
> 
> 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?

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 :-)

-- 
Daniel Jacobowitz
CodeSourcery