Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Mark Kettenis <mark.kettenis@xs4all.nl>
To: eliz@gnu.org
Cc: gdb-patches@sourceware.org
Subject: Re: Save the length of inserted breakpoints
Date: Thu, 13 Apr 2006 22:13:00 -0000	[thread overview]
Message-ID: <200604132213.k3DMDeBX026776@elgar.sibelius.xs4all.nl> (raw)
In-Reply-To: <uvetd3nd7.fsf@gnu.org> (message from Eli Zaretskii on Thu, 13 	Apr 2006 11:12:36 +0300)

> 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


  reply	other threads:[~2006-04-13 22:13 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-03-02 22:25 Daniel Jacobowitz
2006-03-02 23:13 ` Mark Kettenis
2006-03-02 23:19   ` Daniel Jacobowitz
2006-03-03  0:08     ` Mark Kettenis
2006-03-03  1:21       ` Daniel Jacobowitz
2006-03-03 13:51   ` Eli Zaretskii
2006-03-03 15:03     ` Daniel Jacobowitz
2006-03-03 17:56       ` Eli Zaretskii
2006-03-03 18:04         ` Daniel Jacobowitz
2006-03-03 22:00           ` Eli Zaretskii
2006-03-03 22:10             ` Daniel Jacobowitz
2006-03-03 22:35               ` Eli Zaretskii
2006-03-03 23:01                 ` Daniel Jacobowitz
2006-03-04 10:39                   ` Eli Zaretskii
2006-03-04 14:58                     ` Daniel Jacobowitz
2006-03-04 15:05                       ` Eli Zaretskii
2006-03-04 15:11                         ` Daniel Jacobowitz
2006-03-06 19:49                           ` Eli Zaretskii
2006-03-07  5:31               ` Michael Snyder
2006-03-04  0:35             ` Steven Johnson
2006-03-04 10:18               ` Daniel Jacobowitz
2006-04-11 21:46   ` Daniel Jacobowitz
2006-04-11 22:32     ` David S. Miller
2006-04-12  7:30     ` Eli Zaretskii
2006-04-12  9:44     ` Mark Kettenis
2006-04-12 12:57       ` Daniel Jacobowitz
2006-04-12 18:38         ` Mark Kettenis
2006-04-12 18:47           ` Daniel Jacobowitz
2006-04-13  8:12             ` Eli Zaretskii
2006-04-13 22:13               ` Mark Kettenis [this message]
2006-04-13 22:59                 ` Daniel Jacobowitz
2006-04-13 23:30                   ` Daniel Jacobowitz
2006-04-14  8:10                   ` Eli Zaretskii
2006-04-14  8:52                     ` David S. Miller
2006-04-14  8:04                 ` Eli Zaretskii
2006-04-14  8:51                   ` David S. Miller
2006-04-16 23:58                   ` Mark Kettenis
2006-04-17  7:07                     ` Eli Zaretskii
2006-04-13 21:57             ` Michael Snyder
2006-04-13 22:59               ` Daniel Jacobowitz
2006-04-16 23:53                 ` Mark Kettenis
2006-04-16 23:50               ` Mark Kettenis
2006-04-17  1:41                 ` Daniel Jacobowitz
2006-04-17 13:09                   ` Mark Kettenis
2006-04-17 13:37                     ` Daniel Jacobowitz
2006-04-17 13:50                       ` Mark Kettenis
2006-04-17 19:08                         ` Daniel Jacobowitz
2006-04-17 20:25                           ` Mark Kettenis
2006-04-17 21:50                             ` Daniel Jacobowitz
2006-04-18  8:59                           ` Eli Zaretskii
2006-04-18 19:21                             ` Daniel Jacobowitz
2006-04-19  7:40                               ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200604132213.k3DMDeBX026776@elgar.sibelius.xs4all.nl \
    --to=mark.kettenis@xs4all.nl \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox