[PATCH] Print thread name when executing thread commands

[PATCH] Print thread name when executing thread commands

[Pat Pannuto]

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

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.

Bonus: solves my old SO question:
http://stackoverflow.com/questions/12679339/make-gdb-show-thread-names-on-apply-all-operations

-Pat

-- 
Pat Pannuto
NSF/NDSEG/Qualcomm Fellow
Computer Engineering
University of Michigan
248.990.4548

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

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

	* thread.c: Print thread name as well as ID number and PID when
	executing the thread family of commands


[-- 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 06:33:29 -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.  */

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

[Ricard Wanderlof]

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.

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.

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.

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
-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

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

[Pat Pannuto]

[-- 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.  */

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

[Agovic, Sanimir]

Hello Pat,

please have a look at https://sourceware.org/ml/gdb-patches/2013-09/msg00485.html

A question below.

-Sanimir

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf
> Of Pat Pannuto
> Sent: Wednesday, September 18, 2013 08:37 PM
> To: Ricard Wanderlof
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH] Print thread name when executing thread commands
> 
> $ 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]);
>
What happens if a thread changes its name during execution, do we get the 
cached or the actual thread name? It looks to me as we should never directly
access the thread_info::name member, or?

Btw, a testcase would be great.

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

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

[Pat Pannuto]

On Mon, Sep 23, 2013 at 8:18 AM, Agovic, Sanimir
<sanimir.agovic@intel.com> wrote:
> Hello Pat,
>
> please have a look at https://sourceware.org/ml/gdb-patches/2013-09/msg00485.html

Your patch looks superior to mine. Covers more cases I'd missed and
has test cases. I'll happily drop this if the other patch is merged.

It looks like it's been sitting for a while (Mohsan's original patch
went to bugzilla on 2012-09-25..), is there something standing in the
way of the other patch going through? I can try to champion that one
if it's necessary.

>
> A question below.
>
> -Sanimir
>
>> -----Original Message-----
>> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf
>> Of Pat Pannuto
>> Sent: Wednesday, September 18, 2013 08:37 PM
>> To: Ricard Wanderlof
>> Cc: gdb-patches@sourceware.org
>> Subject: Re: [PATCH] Print thread name when executing thread commands
>>
>> $ 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]);
>>
> What happens if a thread changes its name during execution, do we get the
> cached or the actual thread name? It looks to me as we should never directly
> access the thread_info::name member, or?

I honestly don't know. I would expect it would mirror the behavior of
`info threads` as the code is essentially duplicated.

>
> Btw, a testcase would be great.
>
> Intel GmbH
> Dornacher Strasse 1
> 85622 Feldkirchen/Muenchen, Deutschland
> Sitz der Gesellschaft: Feldkirchen bei Muenchen
> Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
> Registergericht: Muenchen HRB 47456
> Ust.-IdNr./VAT Registration No.: DE129385895
> Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052
>



-- 
Pat Pannuto
NSF/NDSEG/Qualcomm Fellow
Computer Engineering
University of Michigan
248.990.4548