[RFC] How to get target_ops from to_kill method?

[Joel Brobecker <brobecker@adacore.com>]

[-- Attachment #1: Type: text/plain, Size: 1406 bytes --]

Hello,

As I was hinting in one of my earlier emails, there is a nasty error
in darwin-nat.c, in the fact that we set the darwin_ops->to_kill method
to a procedure that has the wrong profile: darwin_kill_inferior takes
a target_ops * as its parameter, whereas the to_kill method is supposed
to take none!

Attached is a patch that fixes the issue, at least for now. But as you
can see, darwin_kill_inferior makes calls to a few routines that are
also used as methods in darwin's target_ops, and thus must take
a target_ops as their first parameter.  As a minimal change, what
my first patch does is simply use the current_target global and passes
it to darwin_wait and darwin_resume. But this isn't very satisfactory,
I don't want to increase the use of a global if I can help it.

As it turns out, all the routines being used by darwin_kill_inferior
don't use the target_ops. So what my second patch did was extract out
each of these routines inside another identical function, but without
the target_ops parameter. What this does, basically, is defined the
target_ops methods as wrappers to the real routines. This is what my
second patch does.

However, I'm really wondering whether it would make sense to have
*all* the methods take the target_ops as the first parameter. We've
been slowly adding this parameter as we need them, but really, why
not be consistent across the board?

Thoughts?

-- 
Joel

[-- Attachment #2: darwin-kill.diff --]
[-- Type: text/x-diff, Size: 1344 bytes --]

Index: darwin-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/darwin-nat.c,v
retrieving revision 1.5
diff -u -p -r1.5 darwin-nat.c
--- darwin-nat.c	12 Mar 2009 22:29:30 -0000	1.5
+++ darwin-nat.c	16 Mar 2009 15:53:40 -0000
@@ -90,7 +90,7 @@ static void darwin_mourn_inferior (struc
 
 static int darwin_lookup_task (char *args, task_t * ptask, int *ppid);
 
-static void darwin_kill_inferior (struct target_ops *ops);
+static void darwin_kill_inferior (void);
 
 static void darwin_ptrace_me (void);
 
@@ -367,7 +367,7 @@ darwin_resume (struct target_ops *ops,
 	    {
 	      int nsignal = target_signal_to_host (signal);
 	      res = PTRACE (PT_THUPDATE, pid,
-				   (void *)exc_msg.thread_port, nsignal);
+			    (void *)exc_msg.thread_port, nsignal);
 	      if (res < 0)
 		printf_unfiltered (_("ptrace THUP: res=%d\n"), res);
 	    }
@@ -693,13 +693,16 @@ darwin_stop_inferior (struct target_ops 
 }
 
 static void
-darwin_kill_inferior (struct target_ops *ops)
+darwin_kill_inferior (void)
 {
   struct target_waitstatus wstatus;
   ptid_t ptid;
   kern_return_t kret;
   int status;
   int res;
+  /* FIXME: brobecker/2009-03-16: Is there a better way to get
+     to the current target ops?  */
+  struct target_ops *ops = &current_target;
 
   gdb_assert (darwin_inf != NULL);
 

[-- Attachment #3: darwin-kill2.diff --]
[-- Type: text/x-diff, Size: 3338 bytes --]

Index: darwin-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/darwin-nat.c,v
retrieving revision 1.6
diff -u -p -r1.6 darwin-nat.c
--- darwin-nat.c	16 Mar 2009 15:57:08 -0000	1.6
+++ darwin-nat.c	16 Mar 2009 16:22:01 -0000
@@ -90,8 +90,6 @@ static void darwin_mourn_inferior (struc
 
 static int darwin_lookup_task (char *args, task_t * ptask, int *ppid);
 
-static void darwin_kill_inferior (struct target_ops *ops);
-
 static void darwin_ptrace_me (void);
 
 static void darwin_ptrace_him (int pid);
@@ -337,8 +335,7 @@ darwin_stop (ptid_t t)
 }
 
 static void
-darwin_resume (struct target_ops *ops,
-	       ptid_t ptid, int step, enum target_signal signal)
+darwin_resume_1 (ptid_t ptid, int step, enum target_signal signal)
 {
   struct target_waitstatus status;
   int pid;
@@ -406,6 +403,13 @@ darwin_resume (struct target_ops *ops,
     }
 }
 
+static void
+darwin_resume (struct target_ops *ops,
+	       ptid_t ptid, int step, enum target_signal signal)
+{
+  darwin_resume_1 (ptid, step, signal);
+}
+
 kern_return_t
 catch_exception_raise_state
   (mach_port_t port,
@@ -475,8 +479,7 @@ catch_exception_raise (mach_port_t port,
 }
 
 static ptid_t
-darwin_wait (struct target_ops *ops,
-	     ptid_t ptid, struct target_waitstatus *status)
+darwin_wait_1 (ptid_t ptid, struct target_waitstatus *status)
 {
   kern_return_t kret;
   mach_msg_header_t *hdr = &msgin.hdr;
@@ -613,6 +616,13 @@ darwin_wait (struct target_ops *ops,
     }
 }
 
+static ptid_t
+darwin_wait (struct target_ops *ops,
+	     ptid_t ptid, struct target_waitstatus *status)
+{
+  return darwin_wait_1 (ptid, status);
+}
+
 static void
 darwin_mourn_inferior (struct target_ops *ops)
 {
@@ -668,7 +678,7 @@ darwin_mourn_inferior (struct target_ops
 }
 
 static void
-darwin_stop_inferior (struct target_ops *ops, darwin_inferior *inf)
+darwin_stop_inferior_1 (darwin_inferior *inf)
 {
   struct target_waitstatus wstatus;
   ptid_t ptid;
@@ -688,12 +698,18 @@ darwin_stop_inferior (struct target_ops 
   if (res != 0)
     warning (_("cannot kill: %s\n"), strerror (errno));
 
-  ptid = darwin_wait (ops, inferior_ptid, &wstatus);
+  ptid = darwin_wait_1 (inferior_ptid, &wstatus);
   gdb_assert (wstatus.kind = TARGET_WAITKIND_STOPPED);
 }
 
 static void
-darwin_kill_inferior (struct target_ops *ops)
+darwin_stop_inferior (struct target_ops *ops, darwin_inferior *inf)
+{
+  darwin_stop_inferior_1 (inf);
+}
+
+static void
+darwin_kill_inferior (void)
 {
   struct target_waitstatus wstatus;
   ptid_t ptid;
@@ -706,7 +722,7 @@ darwin_kill_inferior (struct target_ops 
   if (ptid_equal (inferior_ptid, null_ptid))
     return;
 
-  darwin_stop_inferior (ops, darwin_inf);
+  darwin_stop_inferior_1 (darwin_inf);
 
   res = PTRACE (PT_KILL, darwin_inf->pid, 0, 0);
   gdb_assert (res == 0);
@@ -714,13 +730,13 @@ darwin_kill_inferior (struct target_ops 
   if (msg_state == GOT_MESSAGE)
     {
       exc_msg.ex_type = 0;
-      darwin_resume (ops, inferior_ptid, 0, 0);
+      darwin_resume_1 (inferior_ptid, 0, 0);
     }
 
   kret = task_resume (darwin_inf->task);
   MACH_CHECK_ERROR (kret);
 
-  ptid = darwin_wait (ops, inferior_ptid, &wstatus);
+  ptid = darwin_wait_1 (inferior_ptid, &wstatus);
 
   /* This double wait seems required...  */
   res = waitpid (darwin_inf->pid, &status, 0);