This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Can we get rid of go32_stop? (and its normal_stop call?)


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;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]