This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Save the length of inserted breakpoints


> Date: Thu, 13 Apr 2006 11:12:36 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Date: Wed, 12 Apr 2006 14:47:17 -0400
> > From: Daniel Jacobowitz <drow@false.org>
> > Cc: gdb-patches@sourceware.org
> > 
> > > int target_insert_breakpoint(CORE_ADDR addr, gdb_byte *buf, int *size);
> > > int target_remove_breakpoint(CORE_ADDR addr, gdb_byte *buf, int size);
> > 
> > And then if you come up with a reason, you're going to need to hand
> > edit every one of these targets again.  It's not a bundle of fun.  Is
> > that really necessary?

It's a minor effort compared to all the brain cycles spent
understanding a more obfuscated interface.

> > You need an address because the address at which the breakpoint is
> > inserted may not match the requested address.  This happens in several
> > different places in the breakpoint infrastructure (I believe I counted
> > three disjoint hooks for it), but I am particularly looking at
> > BREAKPOINT_FROM_PC, which takes the PC by reference.  In the ARM case,
> > given 0x4001, it strips the low bit off and returns a two byte
> > breakpoint.  If we don't allow the target to save the
> > actually-inserted-at address, then it has to call BREAKPOINT_FROM_PC
> > again.  It feels much more robust to me to save this address when we
> > initially adjust it.  Here's where we inserted the breakpoint, so
> > that's where we should remove it from.
> > 
> > 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.

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

Mark


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]