Re: [PATCH] Print thread name when executing thread commands

[Pat Pannuto <pat.pannuto@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3921 bytes --]

On Wed, Sep 18, 2013 at 3:04 AM, Ricard Wanderlof
<ricard.wanderlof@axis.com> wrote:
>
> On Wed, 18 Sep 2013, Pat Pannuto wrote:
>
>> Currently the thread family of commands only prints the thread ID and
>> PID. GDB also has access to the thread's name, which is often an easier
>> way of quickly identifying a thread. This simple patch uses the same
>> mechanism as `info threads' to get the name of a thread and add it to
>> the thread identifier line when it's printed.
>>
>> I wrapped the name in []'s to offset it and make it visually distinct
>> from the ()'s that wrap the other thread information. I'm not at all
>> attached to this look, just looked best to me.
>>
>> Diff should be from current cvs. This is my first patch to GDB, if I
>> messed something up don't hesitate to yell at me.
>
>
> I'm fairly new here too so maybe I shouldn't say too much, but supplying the
> patch inline (including the ChangeLog entry) in the post makes it much
> easier to comment on it. I still like adding it as an attachement lest the
> mail program messes up the formatting so it doesn't apply.
>

Done.

> My initial comment is that the ChangeLog format requires that it should be
> more specific regarding which functions have been modified. One should be
> able to grep for the function name in the ChangeLog. Something like
>
> 2013-09-18  Pat Pannuto  <pat.pannuto@gmail.com>
>
>         * thread.c (thread_apply_all_command, thread_apply_command):
>         Print thread name as well as ID number and PID.
>

Makes sense. Updated.

> In the code, there was a 'char *name' in one place, but 'char* name' in
> another. Looking quickly, the indentation of multiple-line statements looked
> inconsistent in some places too.

I fixed the misplaced *. As for the spacing, it follows the spacing of
the lines around them AFAICT.

>
> As for the functionality itself, I'm not in a position to approve but I
> think it's a good idea. It's annoying just to get the thread id when there's
> more information available. I don't know if there are any caveats.
>
> /Ricard

Updated ChangeLog, diff:

$ cat print_thread_names.ChangeLog
2013-09-18  Pat Pannuto <pat.pannuto@gmail.com>

    * thread.c (thread_apply_all_command, thread_apply_command):
    Print thread name as well as ID number and PID.


$ cat print_thread_names.diff
Index: gdb/thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.157
diff -u -p -r1.157 thread.c
--- gdb/thread.c    17 Sep 2013 18:26:39 -0000    1.157
+++ gdb/thread.c    18 Sep 2013 18:31:15 -0000
@@ -1255,10 +1255,14 @@ thread_apply_all_command (char *cmd, int
       for (k = 0; k != i; k++)
         if (thread_alive (tp_array[k]))
           {
+            char *name;
             switch_to_thread (tp_array[k]->ptid);
-            printf_filtered (_("\nThread %d (%s):\n"),
+            name = tp_array[k]->name ? tp_array[k]->name
+                    : target_thread_name (tp_array[k]);
+            printf_filtered (_("\nThread %d (%s) [%s]:\n"),
                  tp_array[k]->num,
-                 target_pid_to_str (inferior_ptid));
+                 target_pid_to_str (inferior_ptid),
+                 name);
             execute_command (cmd, from_tty);

             /* Restore exact command used previously.  */
@@ -1308,10 +1312,12 @@ thread_apply_command (char *tidlist, int
     warning (_("Thread %d has terminated."), start);
       else
     {
+      char *name;
       switch_to_thread (tp->ptid);

-      printf_filtered (_("\nThread %d (%s):\n"), tp->num,
-               target_pid_to_str (inferior_ptid));
+      name = tp->name ? tp->name : target_thread_name (tp);
+      printf_filtered (_("\nThread %d (%s) [%s]:\n"), tp->num,
+               target_pid_to_str (inferior_ptid), name);
       execute_command (cmd, from_tty);

       /* Restore exact command used previously.  */

[-- Attachment #2: print_thread_names.ChangeLog --]
[-- Type: application/octet-stream, Size: 161 bytes --]

2013-09-18  Pat Pannuto <pat.pannuto@gmail.com>

	* thread.c (thread_apply_all_command, thread_apply_command):
	Print thread name as well as ID number and PID.


[-- Attachment #3: print_thread_names.diff --]
[-- Type: application/octet-stream, Size: 1513 bytes --]

Index: gdb/thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.157
diff -u -p -r1.157 thread.c
--- gdb/thread.c	17 Sep 2013 18:26:39 -0000	1.157
+++ gdb/thread.c	18 Sep 2013 18:31:15 -0000
@@ -1255,10 +1255,14 @@ thread_apply_all_command (char *cmd, int
       for (k = 0; k != i; k++)
         if (thread_alive (tp_array[k]))
           {
+            char *name;
             switch_to_thread (tp_array[k]->ptid);
-            printf_filtered (_("\nThread %d (%s):\n"), 
+            name = tp_array[k]->name ? tp_array[k]->name
+                    : target_thread_name (tp_array[k]);
+            printf_filtered (_("\nThread %d (%s) [%s]:\n"), 
 			     tp_array[k]->num,
-			     target_pid_to_str (inferior_ptid));
+			     target_pid_to_str (inferior_ptid),
+			     name);
             execute_command (cmd, from_tty);
 
             /* Restore exact command used previously.  */
@@ -1308,10 +1312,12 @@ thread_apply_command (char *tidlist, int
 	warning (_("Thread %d has terminated."), start);
       else
 	{
+	  char *name;
 	  switch_to_thread (tp->ptid);
 
-	  printf_filtered (_("\nThread %d (%s):\n"), tp->num,
-			   target_pid_to_str (inferior_ptid));
+	  name = tp->name ? tp->name : target_thread_name (tp);
+	  printf_filtered (_("\nThread %d (%s) [%s]:\n"), tp->num,
+			   target_pid_to_str (inferior_ptid), name);
 	  execute_command (cmd, from_tty);
 
 	  /* Restore exact command used previously.  */