From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1425 invoked by alias); 16 Mar 2009 16:22:58 -0000 Received: (qmail 1417 invoked by uid 22791); 16 Mar 2009 16:22:56 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 16 Mar 2009 16:22:49 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id D13C52BAB85; Mon, 16 Mar 2009 12:22:47 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id gUcXH4wS1kPf; Mon, 16 Mar 2009 12:22:47 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id A13F32BAB80; Mon, 16 Mar 2009 12:22:47 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 0C024F5C40; Mon, 16 Mar 2009 09:22:47 -0700 (PDT) Date: Mon, 16 Mar 2009 16:42:00 -0000 From: Joel Brobecker To: gdb-patches@sourceware.org Cc: Tristan Gingold Subject: [RFC] How to get target_ops from to_kill method? Message-ID: <20090316162247.GE9576@adacore.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="3O1VwFp74L81IIeR" Content-Disposition: inline User-Agent: Mutt/1.5.18 (2008-05-17) Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-03/txt/msg00265.txt.bz2 --3O1VwFp74L81IIeR Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 1406 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 --3O1VwFp74L81IIeR Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="darwin-kill.diff" Content-length: 1344 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 = ¤t_target; gdb_assert (darwin_inf != NULL); --3O1VwFp74L81IIeR Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="darwin-kill2.diff" Content-length: 3338 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); --3O1VwFp74L81IIeR--