[PATCH] gdb bug 12417

[PATCH] gdb bug 12417

[Mohsan Saleem]

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

Hi,
     Attached patch is for bug 12417, for printing the name of threads while printing the information of a thread. 

Thanks,
Mohsan Saleem

 
--
Mohsan Saleem

[-- Attachment #2: diff.patch --]
[-- Type: application/octet-stream, Size: 5400 bytes --]

diff -ruN ./gdb_old/ChangeLog ./gdb_new/ChangeLog
--- ./gdb_old/ChangeLog	2012-09-24 13:17:34.000000000 +0500
+++ ./gdb_new/ChangeLog	2012-09-24 22:25:44.465441936 +0500
@@ -1,3 +1,12 @@
+2012-09-24  Mohsan Saleem  <mohsansaleem_ms@yahoo.com>
+
+	* thread.c (thread_name): New function. To get thread name
+	(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.
+
 2012-09-21  Steve Ellcey  <sellcey@mips.com>
 
 	* configure.ac: Add mips*-mti-elf* target.
diff -ruN ./gdb_old/gdb/testsuite/gdb.threads/thread-find.exp ./gdb_new/gdb/testsuite/gdb.threads/thread-find.exp
--- ./gdb_old/gdb/testsuite/gdb.threads/thread-find.exp	2012-09-24 13:17:38.000000000 +0500
+++ ./gdb_new/gdb/testsuite/gdb.threads/thread-find.exp	2012-09-24 21:55:44.737537299 +0500
@@ -186,17 +186,17 @@
 
 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 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.
diff -ruN ./gdb_old/gdb/thread.c ./gdb_new/gdb/thread.c
--- ./gdb_old/gdb/thread.c	2012-09-24 13:17:37.000000000 +0500
+++ ./gdb_new/gdb/thread.c	2012-09-24 21:54:46.689540365 +0500
@@ -64,6 +64,7 @@
 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 *);
 
 struct thread_info*
 inferior_thread (void)
@@ -73,6 +74,17 @@
   return tp;
 }
 
+const char*
+thread_name (struct thread_info *ti)
+{
+  const char* name;
+  name = ti->name ? ti->name : target_thread_name(ti);
+  if (name)
+    return name;
+  else
+    return "";
+}
+
 void
 delete_step_resume_breakpoint (struct thread_info *tp)
 {
@@ -236,7 +248,7 @@
   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;
@@ -1198,8 +1210,8 @@
       {
 	switch_to_thread (tp->ptid);
 
-	printf_filtered (_("\nThread %d (%s):\n"),
-			 tp->num, target_pid_to_str (inferior_ptid));
+	printf_filtered (_("\nThread %d %s (%s):\n"), tp->num,
+			 thread_name (tp), target_pid_to_str (inferior_ptid));
 	execute_command (cmd, from_tty);
 	strcpy (cmd, saved_cmd);	/* Restore exact command used
 					   previously.  */
@@ -1250,7 +1262,7 @@
 	{
 	  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);
 
@@ -1347,16 +1359,16 @@
       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++;
 	}
     }
@@ -1397,6 +1409,8 @@
 
   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, thread_name (tp));
   ui_out_text (uiout, " (");
   ui_out_text (uiout, target_pid_to_str (inferior_ptid));
   ui_out_text (uiout, ")]");

Re: [PATCH] gdb bug 12417

[Sergio Durigan Junior]

Hello Mohsan,

Thanks for the patch.  Not really a technical review, but rather
formatting nits.

On Monday, September 24 2012, Mohsan Saleem wrote:

> diff -ruN ./gdb_old/ChangeLog ./gdb_new/ChangeLog
> --- ./gdb_old/ChangeLog	2012-09-24 13:17:34.000000000 +0500
> +++ ./gdb_new/ChangeLog	2012-09-24 22:25:44.465441936 +0500
> @@ -1,3 +1,12 @@
> +2012-09-24  Mohsan Saleem  <mohsansaleem_ms@yahoo.com>
> +
> +	* thread.c (thread_name): New function. To get thread name
                                               ^^

Two spaces after period.  Missing period at the end of the sentence.
You also don't need to explain what `thread_name' does, therefore you
can get rid of the "To get thread name" part.

> +	(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.

Since you are solving a PR, it is recommended that you put its reference
on the ChangeLog message, like this:

2012-09-24  Mohsan Saleem  <mohsansaleem_ms@yahoo.com>

	PR threads/12417
	...
	
> diff -ruN ./gdb_old/gdb/testsuite/gdb.threads/thread-find.exp ./gdb_new/gdb/testsuite/gdb.threads/thread-find.exp
> --- ./gdb_old/gdb/testsuite/gdb.threads/thread-find.exp	2012-09-24 13:17:38.000000000 +0500
> +++ ./gdb_new/gdb/testsuite/gdb.threads/thread-find.exp	2012-09-24 21:55:44.737537299 +0500
> @@ -186,17 +186,17 @@
>  
>  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"

I have some comments about the printing style, see below.

> diff -ruN ./gdb_old/gdb/thread.c ./gdb_new/gdb/thread.c
> --- ./gdb_old/gdb/thread.c	2012-09-24 13:17:37.000000000 +0500
> +++ ./gdb_new/gdb/thread.c	2012-09-24 21:54:46.689540365 +0500
> @@ -64,6 +64,7 @@
>  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 *);
                    ^

The `*' must be together with `thread_name', not `char':

    static const char *thread_name (struct thread_info *);

> +const char*
             ^

Should be:

     const char *

> +thread_name (struct thread_info *ti)
> +{
> +  const char* name;
               ^

Should be:

     const char *name;

> +  name = ti->name ? ti->name : target_thread_name(ti);

The standard is to explict check for NULL.  Also, there must be one
space between the function name and the open paren.

     name = ti->name != NULL ? ti-> name : target_thread_name (ti);

> +  if (name)
> +    return name;
> +  else
> +    return "";

You could do:

    return name != NULL ? name : "";

This whole function could also be shortened, but I think it's OK as-is.

>  void
>  delete_step_resume_breakpoint (struct thread_info *tp)
>  {
> @@ -236,7 +248,7 @@
>    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;
> @@ -1198,8 +1210,8 @@
>        {
>  	switch_to_thread (tp->ptid);
>  
> -	printf_filtered (_("\nThread %d (%s):\n"),
> -			 tp->num, target_pid_to_str (inferior_ptid));
> +	printf_filtered (_("\nThread %d %s (%s):\n"), tp->num,
> +			 thread_name (tp), target_pid_to_str (inferior_ptid));

I would like to hear others' opinions, but I guess this line could be
printed differently.  Currently it is:

        Thread 2 thread_name2 (12323)...

It could now be:

        Thread thread_name2 [#2] (12323)...

I think it gives more importance to the name.  The same comments applies
to all other commands.  But please, don't change anything yet, wait
until a maintainer states his opinions.

WDYT?

-- 
Sergio

gdb bug 12417

[Mohsan Saleem]

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

Hi,
Attached patch is for bug 12417, for printing the name of threads while printing the information of a thread.

Thanks,
Mohsan Saleem 

[-- Attachment #2: 12417.patch --]
[-- Type: application/octet-stream, Size: 3976 bytes --]

--- /root/Desktop/gdb/thread.c	2012-09-03 00:53:02.620254912 -0700
+++ ./gdb/thread.c	2012-09-03 00:58:04.399469614 -0700
@@ -243,12 +243,14 @@
 struct thread_info *
 add_thread_with_info (ptid_t ptid, struct private_thread_info *private)
 {
+  char *name;
   struct thread_info *result = add_thread_silent (ptid);
 
+  name = result->name ? result->name : target_thread_name (result);
   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), name);
 
   annotate_new_thread ();
   return result;
@@ -1192,7 +1194,7 @@
 {
   struct thread_info *tp;
   struct cleanup *old_chain;
-  char *saved_cmd;
+  char *saved_cmd, *name;
 
   if (cmd == NULL || *cmd == '\000')
     error (_("Please specify a command following the thread ID list"));
@@ -1209,8 +1211,9 @@
     if (thread_alive (tp))
       {
 	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, name, target_pid_to_str (inferior_ptid));
 	execute_command (cmd, from_tty);
 	strcpy (cmd, saved_cmd);	/* Restore exact command used
 					   previously.  */
@@ -1222,7 +1225,7 @@
 static void
 thread_apply_command (char *tidlist, int from_tty)
 {
-  char *cmd;
+  char *cmd, *name;
   struct cleanup *old_chain;
   char *saved_cmd;
   struct get_number_or_range_state state;
@@ -1260,8 +1263,8 @@
       else
 	{
 	  switch_to_thread (tp->ptid);
-
-	  printf_filtered (_("\nThread %d (%s):\n"), tp->num,
+	  name = tp->name ? tp->name : target_thread_name (tp);
+	  printf_filtered (_("\nThread %d %s (%s):\n"), tp->num, name,
 			   target_pid_to_str (inferior_ptid));
 	  execute_command (cmd, from_tty);
 
@@ -1283,7 +1286,6 @@
     {
       if (ptid_equal (inferior_ptid, null_ptid))
 	error (_("No thread selected"));
-
       if (target_has_stack)
 	{
 	  if (is_exited (inferior_ptid))
@@ -1327,7 +1329,7 @@
 thread_find_command (char *arg, int from_tty)
 {
   struct thread_info *tp;
-  char *tmp;
+  char *tmp, *name;
   unsigned long match = 0;
 
   if (arg == NULL || *arg == '\0')
@@ -1338,6 +1340,7 @@
     error (_("Invalid regexp (%s): %s"), tmp, arg);
 
   update_thread_list ();
+  
   for (tp = thread_list; tp; tp = tp->next)
     {
       if (tp->name != NULL && re_exec (tp->name))
@@ -1354,20 +1357,20 @@
 			   tp->num, tmp);
 	  match++;
 	}
-
+      name = tp->name ? tp->name : target_thread_name (tp);
       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, name, 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, name, tmp);
 	  match++;
 	}
     }
@@ -1391,6 +1394,7 @@
 {
   int num;
   struct thread_info *tp;
+  char *name;
 
   num = value_as_long (parse_and_eval (tidstr));
 
@@ -1405,9 +1409,12 @@
   switch_to_thread (tp->ptid);
 
   annotate_thread_changed ();
+  name = tp->name ? tp->name : target_thread_name (tp);
 
   ui_out_text (uiout, "[Switching to thread ");
-  ui_out_field_int (uiout, "new-thread-id", pid_to_thread_id (inferior_ptid));
+  ui_out_field_int (uiout, "new-thread-id", pid_to_thread_id (inferior_ptid));
+  ui_out_text (uiout, " ");
+  ui_out_text (uiout, name);
   ui_out_text (uiout, " (");
   ui_out_text (uiout, target_pid_to_str (inferior_ptid));
   ui_out_text (uiout, ")]");

[PATCH] gdb bug 12417

[Mohsan Saleem]

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

Hi,
     Attached patch is for bug 12417, for printing the name of threads while printing the information of a thread. 

Thanks,
Mohsan Saleem 

[-- Attachment #2: 12417.patch --]
[-- Type: application/octet-stream, Size: 3976 bytes --]

--- /root/Desktop/gdb/thread.c	2012-09-03 00:53:02.620254912 -0700
+++ ./gdb/thread.c	2012-09-03 00:58:04.399469614 -0700
@@ -243,12 +243,14 @@
 struct thread_info *
 add_thread_with_info (ptid_t ptid, struct private_thread_info *private)
 {
+  char *name;
   struct thread_info *result = add_thread_silent (ptid);
 
+  name = result->name ? result->name : target_thread_name (result);
   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), name);
 
   annotate_new_thread ();
   return result;
@@ -1192,7 +1194,7 @@
 {
   struct thread_info *tp;
   struct cleanup *old_chain;
-  char *saved_cmd;
+  char *saved_cmd, *name;
 
   if (cmd == NULL || *cmd == '\000')
     error (_("Please specify a command following the thread ID list"));
@@ -1209,8 +1211,9 @@
     if (thread_alive (tp))
       {
 	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, name, target_pid_to_str (inferior_ptid));
 	execute_command (cmd, from_tty);
 	strcpy (cmd, saved_cmd);	/* Restore exact command used
 					   previously.  */
@@ -1222,7 +1225,7 @@
 static void
 thread_apply_command (char *tidlist, int from_tty)
 {
-  char *cmd;
+  char *cmd, *name;
   struct cleanup *old_chain;
   char *saved_cmd;
   struct get_number_or_range_state state;
@@ -1260,8 +1263,8 @@
       else
 	{
 	  switch_to_thread (tp->ptid);
-
-	  printf_filtered (_("\nThread %d (%s):\n"), tp->num,
+	  name = tp->name ? tp->name : target_thread_name (tp);
+	  printf_filtered (_("\nThread %d %s (%s):\n"), tp->num, name,
 			   target_pid_to_str (inferior_ptid));
 	  execute_command (cmd, from_tty);
 
@@ -1283,7 +1286,6 @@
     {
       if (ptid_equal (inferior_ptid, null_ptid))
 	error (_("No thread selected"));
-
       if (target_has_stack)
 	{
 	  if (is_exited (inferior_ptid))
@@ -1327,7 +1329,7 @@
 thread_find_command (char *arg, int from_tty)
 {
   struct thread_info *tp;
-  char *tmp;
+  char *tmp, *name;
   unsigned long match = 0;
 
   if (arg == NULL || *arg == '\0')
@@ -1338,6 +1340,7 @@
     error (_("Invalid regexp (%s): %s"), tmp, arg);
 
   update_thread_list ();
+  
   for (tp = thread_list; tp; tp = tp->next)
     {
       if (tp->name != NULL && re_exec (tp->name))
@@ -1354,20 +1357,20 @@
 			   tp->num, tmp);
 	  match++;
 	}
-
+      name = tp->name ? tp->name : target_thread_name (tp);
       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, name, 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, name, tmp);
 	  match++;
 	}
     }
@@ -1391,6 +1394,7 @@
 {
   int num;
   struct thread_info *tp;
+  char *name;
 
   num = value_as_long (parse_and_eval (tidstr));
 
@@ -1405,9 +1409,12 @@
   switch_to_thread (tp->ptid);
 
   annotate_thread_changed ();
+  name = tp->name ? tp->name : target_thread_name (tp);
 
   ui_out_text (uiout, "[Switching to thread ");
-  ui_out_field_int (uiout, "new-thread-id", pid_to_thread_id (inferior_ptid));
+  ui_out_field_int (uiout, "new-thread-id", pid_to_thread_id (inferior_ptid));
+  ui_out_text (uiout, " ");
+  ui_out_text (uiout, name);
   ui_out_text (uiout, " (");
   ui_out_text (uiout, target_pid_to_str (inferior_ptid));
   ui_out_text (uiout, ")]");

Re: [PATCH] gdb bug 12417

[Jan Kratochvil]

On Wed, 19 Sep 2012 12:59:54 +0200, Mohsan Saleem wrote:
> Attached patch is for bug 12417, for printing the name of threads while
> printing the information of a thread. 

not yet a review but according to the file gdb/CONTRIBUTE there is missing
ChangeLog entry.

And the attachment is base64-encoded with CR-LF (\r\n) there.  It is preferred
to inline the patch as text but be careful not to line-wrap it.  Otherwise
quoted-printable attachment is better (where newlines are implicit).  Or for
base64 at least make it LF.

It would be nice to see some changes in a testcase but it does not have to be.

You should also always run the testsuite (make check) before change and after
the change if there are no regressions.  In this case there are:

-PASS: gdb.threads/thread-find.exp: find thread id 6
-PASS: gdb.threads/thread-find.exp: find thread id 5
-PASS: gdb.threads/thread-find.exp: find thread id 4 
-PASS: gdb.threads/thread-find.exp: find thread id 3 
-PASS: gdb.threads/thread-find.exp: find thread id 2 
-PASS: gdb.threads/thread-find.exp: find thread id 1 
-PASS: gdb.threads/thread-find.exp: find lwp id 6
-PASS: gdb.threads/thread-find.exp: find lwp id 5
-PASS: gdb.threads/thread-find.exp: find lwp id 4
-PASS: gdb.threads/thread-find.exp: find lwp id 3
-PASS: gdb.threads/thread-find.exp: find lwp id 2
-PASS: gdb.threads/thread-find.exp: find lwp id 1
+FAIL: gdb.threads/thread-find.exp: find thread id 6
+FAIL: gdb.threads/thread-find.exp: find thread id 5
+FAIL: gdb.threads/thread-find.exp: find thread id 4
+FAIL: gdb.threads/thread-find.exp: find thread id 3
+FAIL: gdb.threads/thread-find.exp: find thread id 2
+FAIL: gdb.threads/thread-find.exp: find thread id 1
+FAIL: gdb.threads/thread-find.exp: find lwp id 6
+FAIL: gdb.threads/thread-find.exp: find lwp id 5
+FAIL: gdb.threads/thread-find.exp: find lwp id 4
+FAIL: gdb.threads/thread-find.exp: find lwp id 3
+FAIL: gdb.threads/thread-find.exp: find lwp id 2
+FAIL: gdb.threads/thread-find.exp: find lwp id 1
-PASS: gdb.threads/thread_events.exp: continue to threadfunc with messages enabled
+FAIL: gdb.threads/thread_events.exp: continue to threadfunc with messages enabled (saw 0, expected 1)


Thanks,
Jan


> --- /root/Desktop/gdb/thread.c	2012-09-03 00:53:02.620254912 -0700
> +++ ./gdb/thread.c	2012-09-03 00:58:04.399469614 -0700
> @@ -243,12 +243,14 @@

Please use also option -p for diff, it makes the diff more clear.


>  struct thread_info *
>  add_thread_with_info (ptid_t ptid, struct private_thread_info *private)
>  {
> +  char *name;

It could be: const char *name;

>    struct thread_info *result = add_thread_silent (ptid);
>  
> +  name = result->name ? result->name : target_thread_name (result);
>    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), name);

There is excessive space and also the square brackets [ ] should be around the
whole line:
      printf_unfiltered (_("[New %s %s]\n"), target_pid_to_str (ptid), name);


>  
>    annotate_new_thread ();
>    return result;
> @@ -1192,7 +1194,7 @@
>  {
>    struct thread_info *tp;
>    struct cleanup *old_chain;
> -  char *saved_cmd;
> +  char *saved_cmd, *name;

Again const char *.


>  
>    if (cmd == NULL || *cmd == '\000')
>      error (_("Please specify a command following the thread ID list"));
> @@ -1209,8 +1211,9 @@
>      if (thread_alive (tp))
>        {
>  	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, name, target_pid_to_str (inferior_ptid));
>  	execute_command (cmd, from_tty);
>  	strcpy (cmd, saved_cmd);	/* Restore exact command used
>  					   previously.  */

This part does not apply to latest trunk - that is FSF GDB HEAD, please rebase
your patch to it:
	http://www.gnu.org/software/gdb/current/


> @@ -1222,7 +1225,7 @@
>  static void
>  thread_apply_command (char *tidlist, int from_tty)
>  {
> -  char *cmd;
> +  char *cmd, *name;

Again const char *.


>    struct cleanup *old_chain;
>    char *saved_cmd;
>    struct get_number_or_range_state state;
> @@ -1260,8 +1263,8 @@
>        else
>  	{
>  	  switch_to_thread (tp->ptid);
> -
> -	  printf_filtered (_("\nThread %d (%s):\n"), tp->num,
> +	  name = tp->name ? tp->name : target_thread_name (tp);
> +	  printf_filtered (_("\nThread %d %s (%s):\n"), tp->num, name,
>  			   target_pid_to_str (inferior_ptid));
>  	  execute_command (cmd, from_tty);
>  
> @@ -1283,7 +1286,6 @@
>      {
>        if (ptid_equal (inferior_ptid, null_ptid))
>  	error (_("No thread selected"));
> -

Excessive change.


>        if (target_has_stack)
>  	{
>  	  if (is_exited (inferior_ptid))
> @@ -1327,7 +1329,7 @@
>  thread_find_command (char *arg, int from_tty)
>  {
>    struct thread_info *tp;
> -  char *tmp;
> +  char *tmp, *name;

const char *.


>    unsigned long match = 0;
>  
>    if (arg == NULL || *arg == '\0')
> @@ -1338,6 +1340,7 @@
>      error (_("Invalid regexp (%s): %s"), tmp, arg);
>  
>    update_thread_list ();
> +  

Excessive change.


>    for (tp = thread_list; tp; tp = tp->next)
>      {
>        if (tp->name != NULL && re_exec (tp->name))
> @@ -1354,20 +1357,20 @@
>  			   tp->num, tmp);
>  	  match++;
>  	}
> -
Excessive change.


> +      name = tp->name ? tp->name : target_thread_name (tp);
>        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, name, tmp);

The change is incomplete:
(gdb) thread find .*
Thread 2 has target name 'threadit'
Thread 2 threadit has target id 'Thread 0x7ffff7807700 (LWP 31213)'
Thread 1 has target name 'threadit'
Thread 1 threadit has target id 'Thread 0x7ffff7fe5740 (LWP 31207)'

I believe there should be also (although it looks weird):
Thread 2 threadit has target name 'threadit'


Haven't you considered rather changing linux_nat_pid_to_str?

It could be supported also for gdbserver but to_thread_name is currently
already implemented only in linux-nat.c (linux_nat_thread_name) so it is not
a requirement for this patch.


>  	  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, name, tmp);
>  	  match++;
>  	}
>      }
> @@ -1391,6 +1394,7 @@
>  {
>    int num;
>    struct thread_info *tp;
> +  char *name;

const char *


>  
>    num = value_as_long (parse_and_eval (tidstr));
>  
> @@ -1405,9 +1409,12 @@
>    switch_to_thread (tp->ptid);
>  
>    annotate_thread_changed ();
> +  name = tp->name ? tp->name : target_thread_name (tp);
>  
>    ui_out_text (uiout, "[Switching to thread ");
> -  ui_out_field_int (uiout, "new-thread-id", pid_to_thread_id (inferior_ptid));
> +  ui_out_field_int (uiout, "new-thread-id", pid_to_thread_id (inferior_ptid));

There is no change between these lines, I do not understand the diff.


> +  ui_out_text (uiout, " ");
> +  ui_out_text (uiout, name);
>    ui_out_text (uiout, " (");
>    ui_out_text (uiout, target_pid_to_str (inferior_ptid));
>    ui_out_text (uiout, ")]");
> 

Re: [PATCH] gdb bug 12417

[Yao Qi]

On 09/19/2012 06:59 PM, Mohsan Saleem wrote:
> --- /root/Desktop/gdb/thread.c	2012-09-03 00:53:02.620254912 -0700
> +++ ./gdb/thread.c	2012-09-03 00:58:04.399469614 -0700
> @@ -243,12 +243,14 @@
>   struct thread_info *
>   add_thread_with_info (ptid_t ptid, struct private_thread_info *private)
>   {
> +  char *name;
>     struct thread_info *result = add_thread_silent (ptid);
>
> +  name = result->name ? result->name : target_thread_name (result);

This pattern is used five times in your patch, and why don't we move it 
to a function? like,

const char *
thread_info_name (struct thread_info *ti)
{
   return ti->name ? ti->name : target_thread_name (ti);
}

and call this function when needed.

-- 
Yao