Re: [RFC] remote-mips.c: Fix bit rot associated with the inferior's state

[Pedro Alves <pedro at codesourcery dot com>]

On Tuesday 02 March 2010 23:09:20, Kevin Buettner wrote:
> Below is a slight revision over yesterday's patch.   The only difference
> between this patch and the previous one is the addition of the change to
> mips_error().  That change avoids an internal error when inferior_ptid
> is the same as null_ptid.
> 
> I'll wait a few more days for comments before checking this in.
> 
> Kevin
> 
> 	* remote-mips.c (gdbthread.h): Include.
> 	(mips_error): Only mourn the inferior when inferior_ptid is non-null.
> 	(magic_null_ptid): Declare.
> 	(common_open): Set inferior_ptid, add it as an inferior, and
> 	as a thread too.  Set up the inferior's address spaces.
> 	(mips_kill): Make sure that target_mourn_inferior is invoked.
> 	(mips_load): Don't null out inferior_ptid.
> 	(_initialize_remote_mips): Initialize magic_null_ptid.

Looks mostly okay, but are you missing a target_thread_alive
implementation?  See these:

 http://sourceware.org/ml/gdb-patches/2008-08/msg00475.html
 http://sourceware.org/ml/gdb-patches/2008-08/msg00479.html
 http://sourceware.org/ml/gdb-patches/2008-08/msg00531.html

Could you take a look and see if the other changes make
sense to you too?

You may also be interested in this:

 http://sourceware.org/ml/gdb-patches/2008-08/msg00530.html


> 
> Index: remote-mips.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/remote-mips.c,v
> retrieving revision 1.108
> diff -u -p -r1.108 remote-mips.c
> --- remote-mips.c	26 Feb 2010 23:11:24 -0000	1.108
> +++ remote-mips.c	2 Mar 2010 23:02:32 -0000
> @@ -35,6 +35,7 @@
>  #include "regcache.h"
>  #include <ctype.h>
>  #include "mips-tdep.h"
> +#include "gdbthread.h"
>  
>  
>  /* Breakpoint types.  Values 0, 1, and 2 must agree with the watch
> @@ -483,7 +484,8 @@ mips_error (char *string,...)
>    close_ports ();
>  
>    printf_unfiltered ("Ending remote MIPS debugging.\n");
> -  target_mourn_inferior ();
> +  if (!ptid_equal (inferior_ptid, null_ptid))
> +    target_mourn_inferior ();
>  
>    deprecated_throw_reason (RETURN_ERROR);
>  }
> @@ -1457,6 +1459,8 @@ mips_initialize (void)
>    mips_request ('r', 0, 0, &err, mips_receive_wait, NULL);
>  }
>  
> +static ptid_t magic_null_ptid;
> +
>  /* Open a connection to the remote board.  */
>  static void
>  common_open (struct target_ops *ops, char *name, int from_tty,
> @@ -1468,6 +1472,7 @@ common_open (struct target_ops *ops, cha
>    char *remote_name = 0;
>    char *local_name = 0;
>    char **argv;
> +  struct inferior *inf;
>  
>    if (name == 0)
>      error (
> @@ -1563,6 +1568,12 @@ device is attached to the target board (
>    /* Switch to using remote target now.  */
>    push_target (ops);
>  
> +  inferior_ptid = magic_null_ptid;
> +  inf = add_inferior_silent (ptid_get_pid (inferior_ptid));
> +  inf->aspace = current_program_space->aspace;
> +  inf->pspace = current_program_space;
> +  add_thread_silent (inferior_ptid);
> +
>    /* FIXME: Should we call start_remote here?  */
>  
>    /* Try to figure out the processor model if possible.  */
> @@ -2140,7 +2151,10 @@ static void
>  mips_kill (struct target_ops *ops)
>  {
>    if (!mips_wait_flag)
> -    return;
> +    {
> +      target_mourn_inferior ();
> +      return;
> +    }
>  
>    interrupt_count++;
>  
> @@ -2173,6 +2187,8 @@ Give up (and stop debugging it)? ")))
>  
>    serial_send_break (mips_desc);
>  
> +  target_mourn_inferior ();
> +
>  #if 0
>    if (mips_is_open)
>      {
> @@ -3297,8 +3313,6 @@ mips_load (char *file, int from_tty)
>    if (exec_bfd)
>      regcache_write_pc (regcache, bfd_get_start_address (exec_bfd));
>  
> -  inferior_ptid = null_ptid;	/* No process now */
> -
>  /* This is necessary because many things were based on the PC at the time that
>     we attached to the monitor, which is no longer valid now that we have loaded
>     new code (and just changed the PC).  Another way to do this might be to call
> @@ -3458,4 +3472,5 @@ Use \"on\" to enable the masking and \"o
>  			   NULL,
>  			   NULL, /* FIXME: i18n: */
>  			   &setlist, &showlist);
> +  magic_null_ptid = ptid_build (42000, 1, -1);
>  }
> 
> 


-- 
Pedro Alves