From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25681 invoked by alias); 1 May 2009 14:14:37 -0000 Received: (qmail 25671 invoked by uid 22791); 1 May 2009 14:14:36 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00,SARE_SUB_GETRID,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 01 May 2009 14:14:31 +0000 Received: (qmail 28135 invoked from network); 1 May 2009 14:14:29 -0000 Received: from unknown (HELO orlando) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 1 May 2009 14:14:29 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Can we get rid of go32_stop? (and its normal_stop call?) Date: Fri, 01 May 2009 14:14:00 -0000 User-Agent: KMail/1.9.10 Cc: Eli Zaretskii MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200905011514.27632.pedro@codesourcery.com> X-IsSubscribed: yes 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-05/txt/msg00019.txt.bz2 Hi Eli, go32_stop contains a call to normal_stop, that I've been meaning to eliminate for a while --- it is the only call left outside of infrun.c/infcmd.c. Looking at the go32-nat.c code, I think go32_stop has a couple of issues: - go32_stop is registered as target_stop callback, but, target_stop is only useful if the target supports asynchronous execution control --- djgpp doesn't support it. - That means (it looks to me) that the only call to go32_stop that can happen, is from go32_create_inferior, which has this bit: if (prog_has_started) { go32_stop (inferior_ptid); go32_kill_inferior (ops); } AFAICS, this is doing cleanup from a previous run. It seems weird to call go32_kill_inferior here, since go32_create_inferior will only ever be called if the previous inferior is gone already, because the "run" command always kills the previous process. If the previous process exited by itself, then go32_mourn_inferior is called, which itself calls go32_kill_inferior. AFAICS, go32_kill_inferior is always called either when you kill the program or when it exits cleanly, which makes me wonder if we can't remove that bit pasted above, and, make one of go32_kill_inferior or go32_mourn_inferior do the cleanup that go32_stop is doing, clear the prog_has_started flag, and in the process also get rid of the normal_stop call. It also looks strange to have mourn_inferior call kill_inferior, and not the other way around, so I've moved the cleaning up to go32_mourn_inferior. This is completely untested --- I just remembered this since you mentioned go32_stop in the other email, and thought I'd ask and see if I'm in the right direction. -- Pedro Alves 2009-05-01 Pedro Alves * go32-nat.c (go32_stop): Delete. (go32_kill_inferior): Rewrite to only call go32_mourn_inferior. (go32_create_inferior): Don't call go32_stop or go32_kill_inferior. (go32_mourn_inferior): Inline go32_stop and go32_kill_inferior here. (init_go32_ops): Don't register go32_stop. --- gdb/go32-nat.c | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) Index: src/gdb/go32-nat.c =================================================================== --- src.orig/gdb/go32-nat.c 2009-05-01 14:58:43.000000000 +0100 +++ src/gdb/go32-nat.c 2009-05-01 15:04:14.000000000 +0100 @@ -185,7 +185,6 @@ static int go32_xfer_memory (CORE_ADDR m struct mem_attrib *attrib, struct target_ops *target); static void go32_files_info (struct target_ops *target); -static void go32_stop (ptid_t); static void go32_kill_inferior (struct target_ops *ops); static void go32_create_inferior (struct target_ops *ops, char *exec_file, char *args, char **env, int from_tty); @@ -571,25 +570,9 @@ go32_files_info (struct target_ops *targ } static void -go32_stop (ptid_t ptid) -{ - normal_stop (); - cleanup_client (); - ptid = inferior_ptid; - inferior_ptid = null_ptid; - delete_thread_silent (ptid); - prog_has_started = 0; -} - -static void go32_kill_inferior (struct target_ops *ops) { - redir_cmdline_delete (&child_cmd); - resume_signal = -1; - resume_is_step = 0; - if (!ptid_equal (inferior_ptid, null_ptid)) - delete_thread_silent (inferior_ptid); - unpush_target (&go32_ops); + go32_mourn_inferior (ops); } static void @@ -607,11 +590,6 @@ go32_create_inferior (struct target_ops if (exec_file == 0) exec_file = get_exec_file (1); - if (prog_has_started) - { - go32_stop (inferior_ptid); - go32_kill_inferior (ops); - } resume_signal = -1; resume_is_step = 0; @@ -685,6 +663,14 @@ go32_create_inferior (struct target_ops static void go32_mourn_inferior (struct target_ops *ops) { + ptid_t ptid; + + redir_cmdline_delete (&child_cmd); + resume_signal = -1; + resume_is_step = 0; + + cleanup_client (); + /* We need to make sure all the breakpoint enable bits in the DR7 register are reset when the inferior exits. Otherwise, if they rerun the inferior, the uncleared bits may cause random SIGTRAPs, @@ -693,7 +679,13 @@ go32_mourn_inferior (struct target_ops * at all times, but it doesn't, probably under an assumption that the OS cleans up when the debuggee exits. */ i386_cleanup_dregs (); - go32_kill_inferior (ops); + + ptid = inferior_ptid; + inferior_ptid = null_ptid; + delete_thread_silent (ptid); + prog_has_started = 0; + + unpush_target (ops); generic_mourn_inferior (); } @@ -910,7 +902,6 @@ init_go32_ops (void) go32_ops.to_create_inferior = go32_create_inferior; go32_ops.to_mourn_inferior = go32_mourn_inferior; go32_ops.to_can_run = go32_can_run; - go32_ops.to_stop = go32_stop; go32_ops.to_thread_alive = go32_thread_alive; go32_ops.to_pid_to_str = go32_pid_to_str; go32_ops.to_stratum = process_stratum;