From: Pedro Alves <pedro@codesourcery.com>
To: gdb-patches@sourceware.org
Cc: Eli Zaretskii <eliz@gnu.org>
Subject: Can we get rid of go32_stop? (and its normal_stop call?)
Date: Fri, 01 May 2009 14:14:00 -0000 [thread overview]
Message-ID: <200905011514.27632.pedro@codesourcery.com> (raw)
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 <pedro@codesourcery.com>
* 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;
next reply other threads:[~2009-05-01 14:14 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-01 14:14 Pedro Alves [this message]
2009-05-01 15:47 ` Eli Zaretskii
2009-05-01 16:24 ` Pedro Alves
2009-05-01 16:54 ` Eli Zaretskii
2009-05-01 17:50 ` Eli Zaretskii
2009-05-01 17:57 ` Pedro Alves
2009-05-01 17:56 ` Pedro Alves
2009-05-01 17:59 ` DJ Delorie
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=200905011514.27632.pedro@codesourcery.com \
--to=pedro@codesourcery.com \
--cc=eliz@gnu.org \
--cc=gdb-patches@sourceware.org \
/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