Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: pfee@talk21.com
To: Pedro Alves <pedro@codesourcery.com>, gdb-patches@sourceware.org
Cc: Joel Brobecker <brobecker@adacore.com>, Tom Tromey <tromey@redhat.com>
Subject: Re: Enhancement - show old and new thread info when switching during debugging
Date: Tue, 09 Aug 2011 15:25:00 -0000	[thread overview]
Message-ID: <1312903533.54894.YahooMailRC@web86701.mail.ird.yahoo.com> (raw)
In-Reply-To: <1312816830.93830.YahooMailRC@web86703.mail.ird.yahoo.com>

>> Sorry,  but current_inferior_ptid for this will be even more confusing.
>> There's several current_FOO globals that represent the currently selected
>> state.  And one of them is "current_inferior".  Having current_inferior()
>> return one thing, and current_inferior_ptid mean another thing will be a 
>> recipe for long term confusion.
> 
> No problem, I'll pick another name.

I've reverted current_inferior_ptid back to previous_inferior_ptid, which 
makes the patch smaller.

The new variable is now previous_selected_ptid.

>> Any chance you can convince your mailer to  attach patches
>> with "Content-Type: text/x-patch" or some other text  mime
>> type?
> 
> I'm using Yahoo webmail, it's handling of attachments could be better. 
> I'll try using a traditional email client.

I tried kmail which allows setting the content-type and content-disposition, but 
the email didn't reach the gdb-patches list.  I'm reverting to using webmail and 
I'll put the patch as plain text below.

>> Please no leading *  on every comment line.  See other
>> comments, and follow the same  style.
> 
> Ok

Done

> 
>> 
>> In order for this to be accepted, it will need  some
>> documentation in the manual.

Let's consider documentation once the patch is acceptable.

>> 
>> I'm not sure the variable is  sufficiently well defined yet.
>> What does gdb print in this case?
>> 
>>   (gdb) thread 2
>>  (gdb) thread 3
>>  (gdb) p $_prev_thread
>> 
>> It feels like  it should print thread 2, that is, we'd
>> define it to the thread the user last  had selected,
>> and so:
>> 
>>  (gdb) thread $_prev_thread
>>  (gdb) thread  $_prev_thread
>> 
>> would cycle between thread 2 and 3.

I now also set previous_selected_ptid in do_captured_thread_select(), which 
results in the behaviour you've asked for.

>> 
>> This definition  works for non-stop mode as well.
>> Otherwise, as is, we get to define it as  "the thread that
>> the user had selected the last time an execution  command
>> was ran" (and the $_prev_thread is undefined/meaningless  in
>> non-stop mode).
> 
> I appreciate the feedback, your summary of the current operation is
> correct. I'll take a look at making $_prev_thread be influenced by the
> user entering
> "thread" commands.  Since that should then give us consistent operation in
> both all-stop and non-stop modes, the additional change will be
> worthwhile.

I tested the patch in non-stop mode.  $_prev_thread only gets modified when 
the user switches using the "thread" command.  I think that's what you're 
asking for also.

Thanks,
Paul

2011-08-09  Paul Fee <pfee@talk21.com>

    * inferior.h: Add $_prev_thread convenience variable.
    * infrun.c: Add $_prev_thread convenience variable, set it when current 
thread changes.
    * thread.c: Add $_prev_thread convenience variable, set it when user 
switches threads.

Index: inferior.h
===================================================================
RCS file: /cvs/src/src/gdb/inferior.h,v
retrieving revision 1.161
diff -c -p -r1.161 inferior.h
*** inferior.h    21 Jul 2011 23:46:08 -0000    1.161
--- inferior.h    9 Aug 2011 14:44:23 -0000
*************** extern const char *get_inferior_io_termi
*** 94,99 ****
--- 94,105 ----
  
  extern ptid_t inferior_ptid;
  
+ /* Values move from interior_ptid to previous_inferior_ptid to
+  * previous_selected_ptid.  The previous value is exposed to the
+  * user through the $_prev_thread convenience variable.
+  */
+ extern ptid_t previous_selected_ptid;
+ 
  /* Are we simulating synchronous execution? This is used in async gdb
     to implement the 'run', 'continue' etc commands, which will not
     redisplay the prompt until the execution is actually over.  */
*************** extern int stop_on_solib_events;
*** 204,209 ****
--- 210,218 ----
  
  extern void start_remote (int from_tty);
  
+ extern struct value *prev_thread_id_make_value (struct gdbarch *,
+                                                 struct internalvar *);
+ 
  extern void normal_stop (void);
  
  extern int signal_stop_state (int);
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.498
diff -c -p -r1.498 infrun.c
*** infrun.c    4 Aug 2011 19:10:12 -0000    1.498
--- infrun.c    9 Aug 2011 14:44:29 -0000
*************** int sync_execution = 0;
*** 127,132 ****
--- 127,138 ----
  
  static ptid_t previous_inferior_ptid;
  
+ /* Values move from interior_ptid to previous_inferior_ptid to
+    previous_selected_ptid.  The previous selected value is exposed
+    to the user through the $_prev_thread convenience variable.  */
+ 
+ ptid_t previous_selected_ptid;
+ 
  /* Default behavior is to detach newly forked processes (legacy).  */
  int detach_fork = 1;
  
*************** print_no_history_reason (void)
*** 5730,5735 ****
--- 5736,5753 ----
    ui_out_text (current_uiout, "\nNo more reverse-execution history.\n");
  }
  
+ /* Return the previous thread's id.  Return a value of 0 if
+    no previous thread was selected, or it doesn't exist.  */
+ 
+ struct value *
+ prev_thread_id_make_value (struct gdbarch *gdbarch, struct internalvar *var)
+ {
+   struct thread_info *tp = find_thread_ptid (previous_selected_ptid);
+ 
+   return value_from_longest (builtin_type (gdbarch)->builtin_int,
+                  (tp ? tp->num : 0));
+ }
+ 
  /* Here to return control to GDB when the inferior stops for real.
     Print appropriate messages, remove breakpoints, give terminal our modes.
  
*************** normal_stop (void)
*** 5777,5784 ****
      {
        target_terminal_ours_for_output ();
        printf_filtered (_("[Switching to %s]\n"),
!                target_pid_to_str (inferior_ptid));
        annotate_thread_changed ();
        previous_inferior_ptid = inferior_ptid;
      }
  
--- 5795,5803 ----
      {
        target_terminal_ours_for_output ();
        printf_filtered (_("[Switching to %s]\n"),
!                target_pid_to_str (inferior_ptid));
        annotate_thread_changed ();
+       previous_selected_ptid = previous_inferior_ptid;
        previous_inferior_ptid = inferior_ptid;
      }
  
Index: thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.142
diff -c -p -r1.142 thread.c
*** thread.c    4 Aug 2011 19:10:13 -0000    1.142
--- thread.c    9 Aug 2011 14:44:30 -0000
*************** do_captured_thread_select (struct ui_out
*** 1394,1399 ****
--- 1394,1400 ----
    if (!thread_alive (tp))
      error (_("Thread ID %d has terminated."), num);
  
+   previous_selected_ptid = inferior_ptid;
    switch_to_thread (tp->ptid);
  
    annotate_thread_changed ();
*************** Show printing of thread events (such as 
*** 1498,1501 ****
--- 1499,1503 ----
           &setprintlist, &showprintlist);
  
    create_internalvar_type_lazy ("_thread", thread_id_make_value);
+   create_internalvar_type_lazy ("_prev_thread", prev_thread_id_make_value);
  }


  reply	other threads:[~2011-08-09 15:25 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-29 15:29 pfee
2011-07-29 16:01 ` Joel Brobecker
2011-07-29 16:33   ` pfee
2011-07-29 17:38     ` Joel Brobecker
2011-07-29 17:47       ` pfee
2011-07-29 20:40         ` Philippe Waroquiers
2011-07-30  2:44   ` Tom Tromey
2011-07-30 11:39     ` Joel Brobecker
2011-07-30 11:44       ` Pedro Alves
2011-07-30 14:12         ` pfee
2011-08-08 11:35           ` pfee
2011-08-08 15:07             ` Pedro Alves
2011-08-08 15:20               ` pfee
2011-08-09 15:25                 ` pfee [this message]
2011-08-10 15:10                   ` Pedro Alves
2011-08-16 16:01                     ` pfee
2011-07-30 12:40       ` pfee
2011-07-30 11:32 ` Tom Tromey
2011-07-30 22:07 ` Jan Kratochvil

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=1312903533.54894.YahooMailRC@web86701.mail.ird.yahoo.com \
    --to=pfee@talk21.com \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=pedro@codesourcery.com \
    --cc=tromey@redhat.com \
    /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