Re: Save the length of inserted breakpoints

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

> 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?

Mark