This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Get rid of linux-thread-db.c:target_beneath
On Friday 06 February 2009 04:56:58, teawater wrote:
> But we still meet the problem that if a function of beneath target is
> NULL, current target use find_target_beneath to get the beneath
> function pointer is not easy and dangerous like process record meet,
> right?
linux-thread-db.c always sits on top of linux-nat.c currently, so
it is safe to use in the cases I adjusted. When it isn't "safe",
linux-thread-db.c already checks for NULL-ness.
> What about make a interface to support safe beneath function?
For now, you can check that the function pointer is NULL before
calling it. If it is NULL, then try the next target
beneath, etc. See target_attach, target_flash_erase, etc. You
could do something similar in the record.c target. Things
are cleaner on the target method implementation side
if the target method takes a "this" pointer, like the one I'm
adding to target_wait.
If you do that, you get rid of the record_beneath_to_resume,
record_beneath_to_wait, record_beneath_to_store_registers, etc.
function pointer hacks you had. The wrinkle that has, is that your
method ends up responsible for doing the default if no target
implements the method. IMO, that would acceptible for
now. But, it can be fixed.
Now we're mixing up threads, and I was going to start
doing / proposing this separately, but since you are
asking...:
There are diferent "kinds" of target methods.
- those that are defaulted to something that throws an error
say, target_resume calls "noprocess" by default.
- some are always defaulted to something that returns
a reasonable default, or just an ignore function
say, to_fetch_registers, which defaults to "target_ignore".
- some are optional and so default to NULL. Usually in these
cases, the code in target.c checks for NULLness and
continues looking up in the target beneath for a suitable
function pointer.
- some are implemented using inheritance when building up
the current_target target (INHERIT in target.c). On these,
you want to call current_target.foofunc.
- some do the beneath lookup themselves to get away from
this inheritance mechanism.
This is all naturally, a mess. As we talked about before,
a target method should default to calling the same method
on the target beneath. In the bottom of the stack, you have a
sentinel target (the dummy target) that should implement
the default behaviour of the target method.
This way, a target method is never null, so it is always
safe to call the beneath method:
target_foomethod (struct target_ops *ops, ...)
{
beneath = find_target_beneath (ops);
return beneath->foofunc (beneath, ...);
}
See below an example of what I'm talking about.
There a couple of methods were this may get tricky, but
nothing unsurmountable. Notice how doing this also
suggests that target.c/target.h could be half generated
by a target.sh script, similarly to what we do with
gdbarch.{c|h|sh}.
There are few methods in the dummy target, that default
to look up the default run target and run the method there.
This suggests that if we change things like I suggested above,
we need to either make all process_stratum targets implement
those methods, do the lookup beneath stops there, or, make
all process_stratum targets inherit (in the c++ sense) from
inf-child.c or similar; otherwise, e.g, we could be connected
to some remote-like target, while the method would be run on
the native target.
--
Pedro Alves
---
gdb/target.c | 80 +++++++++++++++++++++++++++++++++--------------------------
1 file changed, 45 insertions(+), 35 deletions(-)
Index: src/gdb/target.c
===================================================================
--- src.orig/gdb/target.c 2009-02-06 14:41:50.000000000 +0000
+++ src/gdb/target.c 2009-02-06 15:20:36.000000000 +0000
@@ -376,6 +376,19 @@ default_get_ada_task_ptid (long lwp, lon
return ptid_build (ptid_get_pid (inferior_ptid), lwp, tid);
}
+static ptid_t
+default_to_wait (struct target_ops *ops,
+ ptid_t ptid, struct target_waitstatus *status)
+{
+ return (*ops->beneath->to_wait) (ops, ptid, status);
+}
+
+static void
+default_to_attach (struct target_ops *ops, char *args, int from_tty)
+{
+ (*ops->beneath->to_attach) (ops, args, from_tty);
+}
+
/* Go through the target stack from top to bottom, copying over zero
entries in current_target, then filling in still empty entries. In
effect, we are doing class inheritance through the pushed target
@@ -506,9 +519,11 @@ update_current_target (void)
de_fault (to_close,
(void (*) (int))
target_ignore);
+ de_fault (to_attach, default_to_attach);
de_fault (to_post_attach,
(void (*) (int))
target_ignore);
+ de_fault (to_wait, default_to_wait);
de_fault (to_resume,
(void (*) (ptid_t, int, enum target_signal))
noprocess);
@@ -1848,31 +1863,22 @@ target_disconnect (char *args, int from_
ptid_t
target_wait (ptid_t ptid, struct target_waitstatus *status)
{
- struct target_ops *t;
+ struct target_ops *t = current_target.beneath;
+ ptid_t retval = (t->to_wait) (t, ptid, status);
- for (t = current_target.beneath; t != NULL; t = t->beneath)
+ if (targetdebug)
{
- if (t->to_wait != NULL)
- {
- ptid_t retval = (*t->to_wait) (t, ptid, status);
+ char *status_string;
- if (targetdebug)
- {
- char *status_string;
-
- status_string = target_waitstatus_to_string (status);
- fprintf_unfiltered (gdb_stdlog,
- "target_wait (%d, status) = %d, %s\n",
- PIDGET (ptid), PIDGET (retval),
- status_string);
- xfree (status_string);
- }
-
- return retval;
- }
+ status_string = target_waitstatus_to_string (status);
+ fprintf_unfiltered (gdb_stdlog,
+ "target_wait (%d, status) = %d, %s\n",
+ PIDGET (ptid), PIDGET (retval),
+ status_string);
+ xfree (status_string);
}
- noprocess ();
+ return retval;
}
char *
@@ -2529,6 +2535,19 @@ dummy_pid_to_str (struct target_ops *ops
return normal_pid_to_str (ptid);
}
+static ptid_t
+dummy_to_wait (struct target_ops *ops,
+ ptid_t ptid, struct target_waitstatus *status)
+{
+ noprocess ();
+}
+
+void
+dummy_to_attach (char *args, int from_tty)
+{
+ noprocess ();
+}
+
/* Error-catcher for target_find_memory_regions */
static int dummy_find_memory_regions (int (*ignore1) (), void *ignore2)
{
@@ -2557,6 +2576,7 @@ init_dummy_target (void)
(void (*)(struct target_ops *, char *, int))target_ignore;
dummy_target.to_create_inferior = find_default_create_inferior;
dummy_target.to_can_async_p = find_default_can_async_p;
+ dummy_target.to_wait = dummy_to_wait;
dummy_target.to_is_async_p = find_default_is_async_p;
dummy_target.to_supports_non_stop = find_default_supports_non_stop;
dummy_target.to_pid_to_str = dummy_pid_to_str;
@@ -2590,21 +2610,11 @@ target_close (struct target_ops *targ, i
void
target_attach (char *args, int from_tty)
{
- struct target_ops *t;
- for (t = current_target.beneath; t != NULL; t = t->beneath)
- {
- if (t->to_attach != NULL)
- {
- t->to_attach (t, args, from_tty);
- if (targetdebug)
- fprintf_unfiltered (gdb_stdlog, "target_attach (%s, %d)\n",
- args, from_tty);
- return;
- }
- }
-
- internal_error (__FILE__, __LINE__,
- "could not find a target to attach");
+ struct target_ops *t = current_target.beneath;
+ (*t->to_attach) (t, args, from_tty);
+ if (targetdebug)
+ fprintf_unfiltered (gdb_stdlog, "target_attach (%s, %d)\n",
+ args, from_tty);
}
static void