Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: John Baldwin <jhb@freebsd.org>
To: gdb-patches@sourceware.org
Cc: Pedro Alves <palves@redhat.com>
Subject: Re: [PATCH 1/3] Add fbsd_nat_add_target.
Date: Mon, 27 Apr 2015 19:27:00 -0000	[thread overview]
Message-ID: <2013405.YhOVhnvfYq@ralph.baldwin.cx> (raw)
In-Reply-To: <553E899A.9070105@redhat.com>

On Monday, April 27, 2015 08:10:18 PM Pedro Alves wrote:
> On 04/26/2015 02:24 AM, John Baldwin wrote:
> > Add a wrapper for add_target in fbsd-nat.c to override target operations
> > common to all native FreeBSD targets.
> > 
> > gdb/ChangeLog:
> > 
> > 	* fbsd-nat.c (fbsd_pid_to_exec_file): Mark static.
> > 	(fbsd_find_memory_regions): Mark static.
> > 	(fbsd_nat_add_target): New function.
> > 	* fbsd-nat.h: Export fbsd_nat_add_target and remove prototypes for
> > 	fbsd_pid_to_exec_file and fbsd_find_memory_regions.
> > 	* amd64fbsd-nat.c (_initialize_amd64fbsd_nat): Use fbsd_nat_add_target.
> > 	* i386fbsd-nat.c (_initialize_i386fbsd_nat): Likewise.
> > 	* ppcfbsd-nat.c (_initialize_ppcfbsd_nat): Likewise.
> > 	* sparc64fbsd-nat.c (_initialize_sparc64fbsd_nat): Likewise.
> 
> OOC, any reason you didn't instead do it like:
> 
> struct target_ops *
> fbsd_nat_target (void)
> {
>   struct target_ops *t = inf_ptrace_target ();
> 
>   t->to_pid_to_exec_file = fbsd_pid_to_exec_file;
>   t->to_find_memory_regions = fbsd_find_memory_regions;
>   return t;
> }
> 
> and then use fbsd_nat_target instead of inf_ptrace_target
> directly?
> 
> This maps a little better to a C++ world.
> 
> linux-nat.c does it the way you did as it keeps a separate
> linux_ops target instance around.

I was probably just using linux-nat.c as a reference.  One thing that
confuses me about the linux-nat target is that it keeps linux_ops
around so that it can call the original methods that it overrides,
and yet for a few methods it also uses a local 'super_foo' variable
to call an original method.  I think that those are both doing the
same thing, but perhaps there is some subtlety I'm missing?

I do use a 'super_wait' to call ptrace's wait method in the second
patch in this series, so I could certainly change this to return a
target rather than modifying an existing one if that is preferred.

-- 
John Baldwin


  reply	other threads:[~2015-04-27 19:17 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-26  1:25 John Baldwin
2015-04-27 19:24 ` Pedro Alves
2015-04-27 19:27   ` John Baldwin [this message]
2015-04-27 20:26     ` Mark Kettenis
2015-04-27 20:36       ` John Baldwin
2015-04-28  1:06         ` Mark Kettenis
2015-04-28  1:08           ` Pedro Alves

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=2013405.YhOVhnvfYq@ralph.baldwin.cx \
    --to=jhb@freebsd.org \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    /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