[MI non-stop 01/11] Report thread state in -thread-info output.

[MI non-stop 01/11] Report thread state in -thread-info output.

[Vladimir Prus]

This patch adds the 'state' field to the output of -thread-info. Since
it only changes MI behaviour, I guess I don't need it approved, and
it seems fairly trivial so I doubt we'll have much discussion of this one :-)

- Volodya

	* thread.c (print_thread_info): Add new field "state".
---
 gdb/thread.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/gdb/thread.c b/gdb/thread.c
index d0e17c5..4e172d8 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -806,6 +806,16 @@ print_thread_info (struct ui_out *uiout, int requested_thread)
 			     LOCATION);
 	}
 
+      if (ui_out_is_mi_like_p (uiout))
+	{
+	  char *state = "stopped";
+	  if (tp->state_ == THREAD_EXITED)
+	    state = "exited";
+	  else if (tp->state_ == THREAD_RUNNING)
+	    state = "running";
+	  ui_out_field_string (uiout, "state", state);
+	}
+
       do_cleanups (chain2);
     }
 
-- 
1.5.3.5

Re: [MI non-stop 01/11] Report thread state in -thread-info output.

[Nick Roberts]

 > This patch adds the 'state' field to the output of -thread-info. Since
 > it only changes MI behaviour, I guess I don't need it approved, and
 > it seems fairly trivial so I doubt we'll have much discussion of this one :-)

But this isn't a self contained patch, it seems to need changes in the last
of the series [MI non-stop 10/11].  ISTR that Pedro cascaded his patches so
that it was possible to look at one change after the other.

 > - Volodya
 >
 > 	* thread.c (print_thread_info): Add new field "state".
 > ---
 >  gdb/thread.c |   10 ++++++++++
 >  1 files changed, 10 insertions(+), 0 deletions(-)
 > 
 > diff --git a/gdb/thread.c b/gdb/thread.c
 > index d0e17c5..4e172d8 100644
 > --- a/gdb/thread.c
 > +++ b/gdb/thread.c
 > @@ -806,6 +806,16 @@ print_thread_info (struct ui_out *uiout, int requested_thread)
 >  			     LOCATION);
 >  	}
 >  
 > +      if (ui_out_is_mi_like_p (uiout))
 > +	{
 > +	  char *state = "stopped";
 > +	  if (tp->state_ == THREAD_EXITED)
              ^^^^^^^^^^    ^^^^^^^^^^^^^
Not defined here.

 > +	    state = "exited";
 > +	  else if (tp->state_ == THREAD_RUNNING)
 > +	    state = "running";
 > +	  ui_out_field_string (uiout, "state", state);
 > +	}
 > +
 >        do_cleanups (chain2);
 >      }
 >  
 > -- 
 > 1.5.3.5
 > 
 > 

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [MI non-stop 01/11] Report thread state in -thread-info output.

[Vladimir Prus]

On Sunday 29 June 2008 04:41:59 Nick Roberts wrote:
>  > This patch adds the 'state' field to the output of -thread-info. Since
>  > it only changes MI behaviour, I guess I don't need it approved, and
>  > it seems fairly trivial so I doubt we'll have much discussion of this one :-)
> 
> But this isn't a self contained patch, it seems to need changes in the last
> of the series [MI non-stop 10/11].  

MI non-stop 10/11 is "Skip varobj in running threads.". I don't think there's
any dependency on that. Of course, as I wrote in the overview, this patch needs
Pedro's patches.

> ISTR that Pedro cascaded his patches so 
> that it was possible to look at one change after the other.
> 
>  > - Volodya
>  >
>  > 	* thread.c (print_thread_info): Add new field "state".
>  > ---
>  >  gdb/thread.c |   10 ++++++++++
>  >  1 files changed, 10 insertions(+), 0 deletions(-)
>  > 
>  > diff --git a/gdb/thread.c b/gdb/thread.c
>  > index d0e17c5..4e172d8 100644
>  > --- a/gdb/thread.c
>  > +++ b/gdb/thread.c
>  > @@ -806,6 +806,16 @@ print_thread_info (struct ui_out *uiout, int requested_thread)
>  >  			     LOCATION);
>  >  	}
>  >  
>  > +      if (ui_out_is_mi_like_p (uiout))
>  > +	{
>  > +	  char *state = "stopped";
>  > +	  if (tp->state_ == THREAD_EXITED)
>               ^^^^^^^^^^    ^^^^^^^^^^^^^
> Not defined here.

This field is introduced by Pedro's patches. 

- Volodya

Re: [MI non-stop 01/11] Report thread state in -thread-info output.

[Nick Roberts]

 > > But this isn't a self contained patch, it seems to need changes in the last
 > > of the series [MI non-stop 10/11].  
 > 
 > MI non-stop 10/11 is "Skip varobj in running threads.". I don't think
 > there's any dependency on that. Of course, as I wrote in the overview, this
 > patch needs Pedro's patches.

Yes, you're right it was actually Pedro's patch, "[non-stop] 10/10", that I
had matched on - not yours - sorry about that.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [MI non-stop 01/11] Report thread state in -thread-info output.

[Daniel Jacobowitz]

On Sat, Jun 28, 2008 at 08:34:58PM +0400, Vladimir Prus wrote:
> +	  if (tp->state_ == THREAD_EXITED)
> +	    state = "exited";

  /* This field is internal to thread.c.  Never access it directly,
     use is_running instead.  */
  int state_;

Aren't there accessors you can use here?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [MI non-stop 01/11] Report thread state in -thread-info output.

[Vladimir Prus]

Daniel Jacobowitz wrote:

> On Sat, Jun 28, 2008 at 08:34:58PM +0400, Vladimir Prus wrote:
>> +      if (tp->state_ == THREAD_EXITED)
>> +        state = "exited";
> 
>   /* This field is internal to thread.c.  Never access it directly,
>      use is_running instead.  */
>   int state_;
> 
> Aren't there accessors you can use here?

Well, we talked with Pedro about this, I believe. The issue now is that is_xxx
function take ptid, and do lookup. The right solution would be to have
is_xxx_ptid functions taking ptid and is_xxx functions taking thread_info*.

Pedro, does this approach seem OK for you, in the end?

Meanwhile, I've checked in the patch as posted.

- Volodya

Re: [MI non-stop 01/11] Report thread state in -thread-info output.

[Pedro Alves]

A Sunday 13 July 2008 06:38:50, Vladimir Prus wrote:
> Daniel Jacobowitz wrote:
> > On Sat, Jun 28, 2008 at 08:34:58PM +0400, Vladimir Prus wrote:
> >> +      if (tp->state_ == THREAD_EXITED)
> >> +        state = "exited";
> >
> >   /* This field is internal to thread.c.  Never access it directly,
> >      use is_running instead.  */
> >   int state_;
> >
> > Aren't there accessors you can use here?
>
> Well, we talked with Pedro about this, I believe. The issue now is that
> is_xxx function take ptid, and do lookup. The right solution would be to
> have is_xxx_ptid functions taking ptid and is_xxx functions taking
> thread_info*.
>
> Pedro, does this approach seem OK for you, in the end?

Sure, that, or is_ptid_STATE/is_thread_STATE, or rename state_ to state
and allow access to it directly when we have a thread_info (or adding a
thread_state(thread_info) accessor and make the state enum public).  The
main reason is_$(STATE) were introduced, the member was named state_ and
accessing it directly was noted as forbidden, was because of targets that
don't record the main thread.

Any approach taken is fine with me.

-- 
Pedro Alves