Re: [PATCH] fix PR-12417

[Mohsan Saleem <mohsansaleem_ms@yahoo.com>]

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



 Ping!
--
Mohsan Saleem


----- Forwarded Message -----
From: Mohsan Saleem <mohsansaleem_ms@yahoo.com>
To: "Agovic, Sanimir" <sanimir.agovic@intel.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Sent: Wednesday, 4 September 2013 4:25 PM
Subject: Re: [PATCH] fix PR-12417

gdb\Changlog


2013-09-04  Mohsan Saleem  <msaleem@codesourcery.com>

    PR threads/12417
    * thread.c (thread_name): New function.
    (add_thread_with_info): Update to print thread name.
    (thread_apply_all_command): Likewise.
    (thread_apply_command): Likewise.
    (thread_find_command): Likewise.
    (do_captured_thread_select): Likewise.

 

>Constify TI if possible.
Unable to constify TI. 

>>     if (print_thread_events)
>> -    printf_unfiltered (_("[New %s]\n"), target_pid_to_str (ptid));
>> +    printf_unfiltered (_("[New %s \"%s\"]\n"), target_pid_to_str (ptid), thread_name
>>

>This will print [New 1234 ""] in case a thread name is not available, not sure if we
>should omit thread name instead.

As when a new thread is created then by default program name is assigned to it. So it will print [New 1234 "a.out"], if program is named as "a.out"

>> +{
>> +  const char *name;
>>
>Newline between variable decl and first statement.

Done.



--
Mohsan Saleem


----- Original Message -----
From: "Agovic, Sanimir" <sanimir.agovic@intel.com>
To: 'Mohsan Saleem' <mohsansaleem_ms@yahoo.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Sent: Wednesday, 4 September 2013 1:56 PM
Subject: RE: [PATCH] fix PR-12417

Some comments below. 

-Sanimir

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf
> Of Mohsan Saleem
> Sent: Wednesday, September 04, 2013 10:20 AM
> To: gdb-patches@sourceware.org
> Subject: Re: [PATCH] fix PR-12417
> 
> 
> Index: ./gdb/thread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/thread.c,v
> retrieving revision 1.155
> diff -u -p -r1.155 thread.c
> --- ./gdb/thread.c    15 Jul 2013 11:14:32 -0000    1.155
> +++ ./gdb/thread.c    3 Sep 2013 02:36:11 -0000
> @@ -64,6 +64,7 @@ static void info_threads_command (char *
>   static void thread_apply_command (char *, int);
>   static void restore_current_thread (ptid_t);
>   static void prune_threads (void);
> +static const char *thread_name (struct thread_info *);
> 
>   /* Data to cleanup thread array.  */
> 
> @@ -78,6 +79,14 @@ struct thread_array_cleanup
>   };
> 
> 
> +const char *
> +thread_name (struct thread_info *ti)
>
Constify TI if possible.

> +{
> +  const char *name;
>
Newline between variable decl and first statement.

> +  name = ti->name != NULL ? ti->name : target_thread_name (ti);
>
A matter of taste, but I prefer explicit if/else
name = ti->name;
if (name == NULL) name = target_thread_name (ti);
if (name == NULL) name = "";


> @@ -251,7 +260,7 @@ add_thread_with_info (ptid_t ptid, struc
>     result->private = private;
> 
>     if (print_thread_events)
> -    printf_unfiltered (_("[New %s]\n"), target_pid_to_str (ptid));
> +    printf_unfiltered (_("[New %s \"%s\"]\n"), target_pid_to_str (ptid), thread_name
>
This will print [New 1234 ""] in case a thread name is not available, not sure if we
should omit thread name instead.

> @@ -1456,7 +1466,9 @@ do_captured_thread_select (struct ui_out
> 
>     ui_out_text (uiout, "[Switching to thread ");
>     ui_out_field_int (uiout, "new-thread-id", pid_to_thread_id (inferior_ptid));
> -  ui_out_text (uiout, " (");
> +  ui_out_text (uiout, " \"");
> +  ui_out_text (uiout, thread_name (tp));
> +  ui_out_text (uiout, "\" (");
>     ui_out_text (uiout, target_pid_to_str (inferior_ptid));
>     ui_out_text (uiout, ")]");
> 
Is the thread name exposed via the MI protocol in some way?

-Sanimir
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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 12417.patch --]
[-- Type: text/x-patch; name="12417.patch", Size: 6309 bytes --]

Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.15937
diff -u -p -r1.15937 ChangeLog
--- gdb/ChangeLog	28 Aug 2013 12:25:05 -0000	1.15937
+++ gdb/ChangeLog	4 Sep 2013 11:14:17 -0000
@@ -1,3 +1,13 @@
+2013-08-28  Mohsan Saleem  <msaleem@codesourcery.com>
+
+	PR threads/12417
+	* thread.c (thread_name): New function.
+	(add_thread_with_info): Update to print thread name.
+	(thread_apply_all_command): Likewise.
+	(thread_apply_command): Likewise.
+	(thread_find_command): Likewise.
+	(do_captured_thread_select): Likewise.
+
 2013-08-28  Yao Qi  <yao@codesourcery.com>
 	    Pedro Alves  <palves@redhat.com>
 
Index: gdb/thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.155
diff -u -p -r1.155 thread.c
--- gdb/thread.c	15 Jul 2013 11:14:32 -0000	1.155
+++ gdb/thread.c	4 Sep 2013 11:14:17 -0000
@@ -64,6 +64,7 @@ static void info_threads_command (char *
 static void thread_apply_command (char *, int);
 static void restore_current_thread (ptid_t);
 static void prune_threads (void);
+static const char *thread_name (struct thread_info *);
 
 /* Data to cleanup thread array.  */
 
@@ -78,6 +79,15 @@ struct thread_array_cleanup
 };
 
 
+const char *
+thread_name (struct thread_info *ti)
+{
+  const char *name;
+
+  name = ti->name != NULL ? ti->name : target_thread_name (ti);
+  return name != NULL ? name : "";
+}
+
 struct thread_info*
 inferior_thread (void)
 {
@@ -251,7 +261,7 @@ add_thread_with_info (ptid_t ptid, struc
   result->private = private;
 
   if (print_thread_events)
-    printf_unfiltered (_("[New %s]\n"), target_pid_to_str (ptid));
+    printf_unfiltered (_("[New %s \"%s\"]\n"), target_pid_to_str (ptid), thread_name (result));
 
   annotate_new_thread ();
   return result;
@@ -1256,9 +1266,10 @@ thread_apply_all_command (char *cmd, int
         if (thread_alive (tp_array[k]))
           {
             switch_to_thread (tp_array[k]->ptid);
-            printf_filtered (_("\nThread %d (%s):\n"), 
-			     tp_array[k]->num,
-			     target_pid_to_str (inferior_ptid));
+            printf_filtered (_("\nThread %d \"%s\" (%s):\n"),
+                             tp_array[k]->num,
+                             thread_name (tp_array[k]),
+                             target_pid_to_str (inferior_ptid));
             execute_command (cmd, from_tty);
 
             /* Restore exact command used previously.  */
@@ -1310,7 +1321,7 @@ thread_apply_command (char *tidlist, int
 	{
 	  switch_to_thread (tp->ptid);
 
-	  printf_filtered (_("\nThread %d (%s):\n"), tp->num,
+	  printf_filtered (_("\nThread %d \"%s\" (%s):\n"), tp->num, thread_name (tp),
 			   target_pid_to_str (inferior_ptid));
 	  execute_command (cmd, from_tty);
 
@@ -1406,16 +1417,16 @@ thread_find_command (char *arg, int from
       tmp = target_pid_to_str (tp->ptid);
       if (tmp != NULL && re_exec (tmp))
 	{
-	  printf_filtered (_("Thread %d has target id '%s'\n"),
-			   tp->num, tmp);
+          printf_filtered (_("Thread %d \"%s\" has target id '%s'\n"),
+			   tp->num, thread_name (tp), tmp);
 	  match++;
 	}
 
       tmp = target_extra_thread_info (tp);
       if (tmp != NULL && re_exec (tmp))
 	{
-	  printf_filtered (_("Thread %d has extra info '%s'\n"),
-			   tp->num, tmp);
+	  printf_filtered (_("Thread %d \"%s\" has extra info '%s'\n"),
+			   tp->num, thread_name (tp), tmp);
 	  match++;
 	}
     }
@@ -1456,7 +1467,9 @@ do_captured_thread_select (struct ui_out
 
   ui_out_text (uiout, "[Switching to thread ");
   ui_out_field_int (uiout, "new-thread-id", pid_to_thread_id (inferior_ptid));
-  ui_out_text (uiout, " (");
+  ui_out_text (uiout, " \"");
+  ui_out_text (uiout, thread_name (tp));
+  ui_out_text (uiout, "\" (");
   ui_out_text (uiout, target_pid_to_str (inferior_ptid));
   ui_out_text (uiout, ")]");
 
Index: gdb/testsuite/gdb.threads/thread-find.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.threads/thread-find.exp,v
retrieving revision 1.9
diff -u -p -r1.9 thread-find.exp
--- gdb/testsuite/gdb.threads/thread-find.exp	1 Jan 2013 06:41:27 -0000	1.9
+++ gdb/testsuite/gdb.threads/thread-find.exp	4 Sep 2013 11:14:17 -0000
@@ -186,17 +186,17 @@ gdb_test "thread find threadname_1" \
 
 if { [info exists thread6] } then {
     gdb_test "thread find $thread6" \
-	"Thread 6 has .*$thread6.*" "find thread id 6"
+	"Thread 6 \"threadname_6\" has .*$thread6.*" "find thread id 6"
     gdb_test "thread find $thread5" \
-	"Thread 5 has .*$thread5.*" "find thread id 5"
+	"Thread 5 \"threadname_5\" has .*$thread5.*" "find thread id 5"
     gdb_test "thread find $thread4" \
-	"Thread 4 has .*$thread4.*" "find thread id 4"
+	"Thread 4 \"threadname_4\" has .*$thread4.*" "find thread id 4"
     gdb_test "thread find $thread3" \
-	"Thread 3 has .*$thread3.*" "find thread id 3"
+	"Thread 3 \"threadname_3\" has .*$thread3.*" "find thread id 3"
     gdb_test "thread find $thread2" \
-	"Thread 2 has .*$thread2.*" "find thread id 2"
+	"Thread 2 \"threadname_2\" has .*$thread2.*" "find thread id 2"
     gdb_test "thread find $thread1" \
-	"Thread 1 has .*$thread1.*" "find thread id 1"
+	"Thread 1 \"threadname_1\" has .*$thread1.*" "find thread id 1"
 }
 
 #
@@ -224,17 +224,17 @@ if { [info exists process6] } then {
 
 if { [info exists lwp6] } then {
     gdb_test "thread find $lwp6" \
-	"Thread 6 has .*$lwp6.*" "find lwp id 6"
+	"Thread 6 \"threadname_6\" has .*$lwp6.*" "find lwp id 6"
     gdb_test "thread find $lwp5" \
-	"Thread 5 has .*$lwp5.*" "find lwp id 5"
+	"Thread 5 \"threadname_5\" has .*$lwp5.*" "find lwp id 5"
     gdb_test "thread find $lwp4" \
-	"Thread 4 has .*$lwp4.*" "find lwp id 4"
+	"Thread 4 \"threadname_4\" has .*$lwp4.*" "find lwp id 4"
     gdb_test "thread find $lwp3" \
-	"Thread 3 has .*$lwp3.*" "find lwp id 3"
+	"Thread 3 \"threadname_3\" has .*$lwp3.*" "find lwp id 3"
     gdb_test "thread find $lwp2" \
-	"Thread 2 has .*$lwp2.*" "find lwp id 2"
+	"Thread 2 \"threadname_2\" has .*$lwp2.*" "find lwp id 2"
     gdb_test "thread find $lwp1" \
-	"Thread 1 has .*$lwp1.*" "find lwp id 1"
+	"Thread 1 \"threadname_1\" has .*$lwp1.*" "find lwp id 1"
 }
 
 # Test no match.