[RFA] Inform about new thread in a single place.

[RFA] Inform about new thread in a single place.

[Vladimir Prus]

We have lots of place where "[New thread XXX]" is printed.
This patch makes add_thread responsible for that. OK?

- Volodya


	Inform about new thread in a single place.
	* thread.c (add_thread): Announce new thread.
	* inf-ttrace.c (inf_ttrace_wait): Don't
	inform about new thread, as add_thread is always
	called too, and will take care of that.
	* infrun.c (handle_inferior_event): Likewise.
	* procfs.c (procfs_wait): Likewise.
	* remote.c (remote_currthread): Likewise.
	* sol-thread.c (sol_thread_wait): Likewise.
	* win32-nat.c (get_win32_debug_event): Likewise.
	* gdb/linux-nat.c (lin_lwp_attach_lwp): Likewise.
	* linux-thread-db.c (attach_thread): Don't inform
	about new thread.  This is called only from
	linux-thread-db.c:attach_thread, which will take care.
---
 gdb/inf-ttrace.c      |    1 -
 gdb/infrun.c          |    8 +-------
 gdb/linux-nat.c       |    5 -----
 gdb/linux-thread-db.c |    3 ---
 gdb/procfs.c          |   14 +++-----------
 gdb/remote.c          |    7 +------
 gdb/sol-thread.c      |    5 +----
 gdb/thread.c          |    3 +++
 gdb/win32-nat.c       |    4 ----
 9 files changed, 9 insertions(+), 41 deletions(-)

diff --git a/gdb/inf-ttrace.c b/gdb/inf-ttrace.c
index 420133a..ffbfe9c 100644
--- a/gdb/inf-ttrace.c
+++ b/gdb/inf-ttrace.c
@@ -962,7 +962,6 @@ inf_ttrace_wait (ptid_t ptid, struct target_waitstatus *ourstatus)
 		  sizeof (struct inf_ttrace_private_thread_info));
 	  inf_ttrace_num_lwps++;
 	}
-      printf_filtered (_("[New %s]\n"), target_pid_to_str (ptid));
       ti = add_thread (ptid);
       ti->private =
 	xmalloc (sizeof (struct inf_ttrace_private_thread_info));
diff --git a/gdb/infrun.c b/gdb/infrun.c
index e651425..1cd5e98 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1339,13 +1339,7 @@ handle_inferior_event (struct execution_control_state *ecs)
 
   if (ecs->ws.kind != TARGET_WAITKIND_EXITED
       && ecs->ws.kind != TARGET_WAITKIND_SIGNALLED && ecs->new_thread_event)
-    {
-      add_thread (ecs->ptid);
-
-      ui_out_text (uiout, "[New ");
-      ui_out_text (uiout, target_pid_to_str (ecs->ptid));
-      ui_out_text (uiout, "]\n");
-    }
+    add_thread (ecs->ptid);
 
   switch (ecs->ws.kind)
     {
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 9a39ab6..fa8cedb 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -955,9 +955,6 @@ lin_lwp_attach_lwp (ptid_t ptid, int verbose)
       lp->stopped = 1;
     }
 
-  if (verbose)
-    printf_filtered (_("[New %s]\n"), target_pid_to_str (ptid));
-
   return 0;
 }
 
@@ -2089,8 +2086,6 @@ retry:
 		}
 
 	      add_thread (lp->ptid);
-	      printf_unfiltered (_("[New %s]\n"),
-				 target_pid_to_str (lp->ptid));
 	    }
 
 	  /* Save the trap's siginfo in case we need it later.  */
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index d2398df..16c223b 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -710,9 +710,6 @@ attach_thread (ptid_t ptid, const td_thrhandle_t *th_p,
   tp->private = xmalloc (sizeof (struct private_thread_info));
   memset (tp->private, 0, sizeof (struct private_thread_info));
 
-  if (verbose)
-    printf_unfiltered (_("[New %s]\n"), target_pid_to_str (ptid));
-
   /* Enable thread event reporting for this thread.  */
   err = td_thr_event_enable_p (th_p, 1);
   if (err != TD_OK)
diff --git a/gdb/procfs.c b/gdb/procfs.c
index e288459..2cac76a 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -4096,11 +4096,8 @@ wait_again:
 		    temp_ptid = MERGEPID (pi->pid, temp_tid);
 		    /* If not in GDB's thread list, add it.  */
 		    if (!in_thread_list (temp_ptid))
-		      {
-			printf_filtered (_("[New %s]\n"),
-					 target_pid_to_str (temp_ptid));
-			add_thread (temp_ptid);
-		      }
+		      add_thread (temp_ptid);
+
 		    /* Return to WFI, but tell it to immediately resume. */
 		    status->kind = TARGET_WAITKIND_SPURIOUS;
 		    return inferior_ptid;
@@ -4166,11 +4163,7 @@ wait_again:
 		    /* If not in GDB's thread list, add it.  */
 		    temp_ptid = MERGEPID (pi->pid, temp_tid);
 		    if (!in_thread_list (temp_ptid))
-		      {
-			printf_filtered (_("[New %s]\n"),
-					 target_pid_to_str (temp_ptid));
-			add_thread (temp_ptid);
-		      }
+		      add_thread (temp_ptid);
 
 		    status->kind = TARGET_WAITKIND_STOPPED;
 		    status->value.sig = 0;
@@ -4257,7 +4250,6 @@ wait_again:
 		   * If we don't create a procinfo, resume may be unhappy
 		   * later.
 		   */
-		  printf_filtered (_("[New %s]\n"), target_pid_to_str (retval));
 		  add_thread (retval);
 		  if (find_procinfo (PIDGET (retval), TIDGET (retval)) == NULL)
 		    create_procinfo (PIDGET (retval), TIDGET (retval));
diff --git a/gdb/remote.c b/gdb/remote.c
index 452af07..69faedb 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1049,12 +1049,7 @@ record_currthread (int currthread)
   /* If this is a new thread, add it to GDB's thread list.
      If we leave it up to WFI to do this, bad things will happen.  */
   if (!in_thread_list (pid_to_ptid (currthread)))
-    {
-      add_thread (pid_to_ptid (currthread));
-      ui_out_text (uiout, "[New ");
-      ui_out_text (uiout, target_pid_to_str (pid_to_ptid (currthread)));
-      ui_out_text (uiout, "]\n");
-    }
+    add_thread (pid_to_ptid (currthread));
 }
 
 static char *last_pass_packet;
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index bf40696..c139397 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -461,10 +461,7 @@ sol_thread_wait (ptid_t ptid, struct target_waitstatus *ourstatus)
       if (is_thread (rtnval)
 	  && !ptid_equal (rtnval, save_ptid)
 	  && !in_thread_list (rtnval))
-	{
-	  printf_filtered ("[New %s]\n", target_pid_to_str (rtnval));
-	  add_thread (rtnval);
-	}
+	add_thread (rtnval);
     }
 
   /* During process initialization, we may get here without the thread
diff --git a/gdb/thread.c b/gdb/thread.c
index 3c8644b..a7861cb 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -130,6 +130,9 @@ add_thread (ptid_t ptid)
   tp->num = ++highest_thread_num;
   tp->next = thread_list;
   thread_list = tp;
+
+  printf_filtered (_("[New %s]\n"), target_pid_to_str (ptid));
+
   return tp;
 }
 
diff --git a/gdb/win32-nat.c b/gdb/win32-nat.c
index 07ebef9..9818839 100644
--- a/gdb/win32-nat.c
+++ b/gdb/win32-nat.c
@@ -1317,10 +1317,6 @@ get_win32_debug_event (int pid, struct target_waitstatus *ourstatus)
       /* Record the existence of this thread */
       th = win32_add_thread (current_event.dwThreadId,
 			     current_event.u.CreateThread.hThread);
-      if (info_verbose)
-	printf_unfiltered ("[New %s]\n",
-			   target_pid_to_str (
-			     pid_to_ptid (current_event.dwThreadId)));
       retval = current_event.dwThreadId;
       break;
 
-- 
1.5.3.5

Re: [RFA] Inform about new thread in a single place.

[Mark Kettenis]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Fri, 14 Dec 2007 20:09:08 +0300
> 
> We have lots of place where "[New thread XXX]" is printed.
> This patch makes add_thread responsible for that. OK?

No, there are places in the code where add_thread() is called where we
should not print the "[New thread xxx]" message.  For example in
bsd-uthread.c, where the main thread is added to the list of threads
to associate process ID and thread ID.

> 	Inform about new thread in a single place.
> 	* thread.c (add_thread): Announce new thread.
> 	* inf-ttrace.c (inf_ttrace_wait): Don't
> 	inform about new thread, as add_thread is always
> 	called too, and will take care of that.
> 	* infrun.c (handle_inferior_event): Likewise.
> 	* procfs.c (procfs_wait): Likewise.
> 	* remote.c (remote_currthread): Likewise.
> 	* sol-thread.c (sol_thread_wait): Likewise.
> 	* win32-nat.c (get_win32_debug_event): Likewise.
> 	* gdb/linux-nat.c (lin_lwp_attach_lwp): Likewise.
> 	* linux-thread-db.c (attach_thread): Don't inform
> 	about new thread.  This is called only from
> 	linux-thread-db.c:attach_thread, which will take care.

Re: [RFA] Inform about new thread in a single place.

[Vladimir Prus]

On Friday 14 December 2007 21:47:45 Mark Kettenis wrote:
> > From: Vladimir Prus <vladimir@codesourcery.com>
> > Date: Fri, 14 Dec 2007 20:09:08 +0300
> > 
> > We have lots of place where "[New thread XXX]" is printed.
> > This patch makes add_thread responsible for that. OK?
> 
> No, there are places in the code where add_thread() is called where we
> should not print the "[New thread xxx]" message.  For example in
> bsd-uthread.c, where the main thread is added to the list of threads
> to associate process ID and thread ID.

Do you mean this code:

  /* HACK: Twiddle INFERIOR_PTID such that the initial thread of a
     process isn't recognized as a new thread.  */
  if (ptid_get_tid (ptid) != 0 && !in_thread_list (ptid)
      && ptid_get_tid (inferior_ptid) == 0)
    {
      add_thread (ptid);
      inferior_ptid = ptid;
    }

? I've just re-checked, and it appears that:

1. The modules that called add_thread without printing
a message about new thread, prior to my patch are:

	- aix-thread.c
	- bsd-uthread.c
	- corelow.c
	- hpux-thread.c
	- nto-procfs.c
	
Looking at those, it seems like the place you've pointed
at is the only one where reporting of new thread is undesirable.

What about me adding 'add_thread_silent' function that will
not print any message, and using it in bds-uthread.c?

- Volodya

Re: [RFA] Inform about new thread in a single place.

[Mark Kettenis]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Fri, 14 Dec 2007 22:07:19 +0300
> 
> On Friday 14 December 2007 21:47:45 Mark Kettenis wrote:
> > > From: Vladimir Prus <vladimir@codesourcery.com>
> > > Date: Fri, 14 Dec 2007 20:09:08 +0300
> > > 
> > > We have lots of place where "[New thread XXX]" is printed.
> > > This patch makes add_thread responsible for that. OK?
> > 
> > No, there are places in the code where add_thread() is called where we
> > should not print the "[New thread xxx]" message.  For example in
> > bsd-uthread.c, where the main thread is added to the list of threads
> > to associate process ID and thread ID.
> 
> Do you mean this code:
> 
>   /* HACK: Twiddle INFERIOR_PTID such that the initial thread of a
>      process isn't recognized as a new thread.  */
>   if (ptid_get_tid (ptid) != 0 && !in_thread_list (ptid)
>       && ptid_get_tid (inferior_ptid) == 0)
>     {
>       add_thread (ptid);
>       inferior_ptid = ptid;
>     }
> 
> ? I've just re-checked, and it appears that:

Yes.

> 1. The modules that called add_thread without printing
> a message about new thread, prior to my patch are:
> 
> 	- aix-thread.c
> 	- bsd-uthread.c
> 	- corelow.c
> 	- hpux-thread.c
> 	- nto-procfs.c
> 	
> Looking at those, it seems like the place you've pointed
> at is the only one where reporting of new thread is undesirable.
> 
> What about me adding 'add_thread_silent' function that will
> not print any message, and using it in bds-uthread.c?

That'd be acceptable to me.  I'm not absolutely true that none of the
modules you mention above don't need this, but we can fix those when
we notice the problem.

Mark

Re: [RFA] Inform about new thread in a single place.

[Vladimir Prus]

On Friday 14 December 2007 22:20:31 you wrote:
> > From: Vladimir Prus <vladimir@codesourcery.com>
> > Date: Fri, 14 Dec 2007 22:07:19 +0300
> > 
> > On Friday 14 December 2007 21:47:45 Mark Kettenis wrote:
> > > > From: Vladimir Prus <vladimir@codesourcery.com>
> > > > Date: Fri, 14 Dec 2007 20:09:08 +0300
> > > > 
> > > > We have lots of place where "[New thread XXX]" is printed.
> > > > This patch makes add_thread responsible for that. OK?
> > > 
> > > No, there are places in the code where add_thread() is called where we
> > > should not print the "[New thread xxx]" message.  For example in
> > > bsd-uthread.c, where the main thread is added to the list of threads
> > > to associate process ID and thread ID.
> > 
> > Do you mean this code:
> > 
> >   /* HACK: Twiddle INFERIOR_PTID such that the initial thread of a
> >      process isn't recognized as a new thread.  */
> >   if (ptid_get_tid (ptid) != 0 && !in_thread_list (ptid)
> >       && ptid_get_tid (inferior_ptid) == 0)
> >     {
> >       add_thread (ptid);
> >       inferior_ptid = ptid;
> >     }
> > 
> > ? I've just re-checked, and it appears that:
> 
> Yes.
> 
> > 1. The modules that called add_thread without printing
> > a message about new thread, prior to my patch are:
> > 
> > 	- aix-thread.c
> > 	- bsd-uthread.c
> > 	- corelow.c
> > 	- hpux-thread.c
> > 	- nto-procfs.c
> > 	
> > Looking at those, it seems like the place you've pointed
> > at is the only one where reporting of new thread is undesirable.
> > 
> > What about me adding 'add_thread_silent' function that will
> > not print any message, and using it in bds-uthread.c?
> 
> That'd be acceptable to me.  I'm not absolutely true that none of the
> modules you mention above don't need this, but we can fix those when
> we notice the problem.

Here's the revised patch. OK?

- Volodya

	* thread.c (add_thread_silent): Renamed
	from add_thread.
	(add_thread): Announce new thread.
	* gdbthread.h (add_thread_silent): Declare.
	* inf-ttrace.c (inf_ttrace_wait): Don't
	inform about new thread, as add_thread is always
	called too, and will take care of that.
	* infrun.c (handle_inferior_event): Likewise.
	* procfs.c (procfs_wait): Likewise.
	* remote.c (remote_currthread): Likewise.
	* sol-thread.c (sol_thread_wait): Likewise.
	* win32-nat.c (get_win32_debug_event): Likewise.
	* gdb/linux-nat.c (lin_lwp_attach_lwp): Likewise.
	* linux-thread-db.c (attach_thread): Don't inform
	about new thread.  This is called only from
	linux-thread-db.c:attach_thread, which will take care.
---
 gdb/bsd-uthread.c     |    2 +-
 gdb/gdbthread.h       |   11 ++++++++---
 gdb/inf-ttrace.c      |    1 -
 gdb/infrun.c          |    8 +-------
 gdb/linux-nat.c       |    5 -----
 gdb/linux-thread-db.c |    3 ---
 gdb/procfs.c          |   14 +++-----------
 gdb/remote.c          |    7 +------
 gdb/sol-thread.c      |    5 +----
 gdb/thread.c          |   16 ++++++++++++----
 gdb/win32-nat.c       |    4 ----
 11 files changed, 27 insertions(+), 49 deletions(-)

diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
index 3c8714a..064b7de 100644
--- a/gdb/bsd-uthread.c
+++ b/gdb/bsd-uthread.c
@@ -366,7 +366,7 @@ bsd_uthread_wait (ptid_t ptid, struct target_waitstatus *status)
   if (ptid_get_tid (ptid) != 0 && !in_thread_list (ptid)
       && ptid_get_tid (inferior_ptid) == 0)
     {
-      add_thread (ptid);
+      add_thread_silent (ptid);
       inferior_ptid = ptid;
     }
 
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 12e0bcc..403e7e3 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -69,11 +69,16 @@ struct thread_info
 /* Create an empty thread list, or empty the existing one.  */
 extern void init_thread_list (void);
 
-/* Add a thread to the thread list.
-   Note that add_thread now returns the handle of the new thread,
-   so that the caller may initialize the private thread data.  */
+/* Add a thread to the thread list, print a message
+   that a new thread is found, and return the pointer to
+   the new thread.  Caller my use this pointer to 
+   initialize the private thread data.  */
 extern struct thread_info *add_thread (ptid_t ptid);
 
+/* Same as add_thread, but does not print a message
+   about new thread.  */
+extern struct thread_info *add_thread_silent (ptid_t ptid);
+
 /* Delete an existing thread list entry.  */
 extern void delete_thread (ptid_t);
 
diff --git a/gdb/inf-ttrace.c b/gdb/inf-ttrace.c
index 420133a..ffbfe9c 100644
--- a/gdb/inf-ttrace.c
+++ b/gdb/inf-ttrace.c
@@ -962,7 +962,6 @@ inf_ttrace_wait (ptid_t ptid, struct target_waitstatus *ourstatus)
 		  sizeof (struct inf_ttrace_private_thread_info));
 	  inf_ttrace_num_lwps++;
 	}
-      printf_filtered (_("[New %s]\n"), target_pid_to_str (ptid));
       ti = add_thread (ptid);
       ti->private =
 	xmalloc (sizeof (struct inf_ttrace_private_thread_info));
diff --git a/gdb/infrun.c b/gdb/infrun.c
index e651425..1cd5e98 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1339,13 +1339,7 @@ handle_inferior_event (struct execution_control_state *ecs)
 
   if (ecs->ws.kind != TARGET_WAITKIND_EXITED
       && ecs->ws.kind != TARGET_WAITKIND_SIGNALLED && ecs->new_thread_event)
-    {
-      add_thread (ecs->ptid);
-
-      ui_out_text (uiout, "[New ");
-      ui_out_text (uiout, target_pid_to_str (ecs->ptid));
-      ui_out_text (uiout, "]\n");
-    }
+    add_thread (ecs->ptid);
 
   switch (ecs->ws.kind)
     {
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 9a39ab6..fa8cedb 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -955,9 +955,6 @@ lin_lwp_attach_lwp (ptid_t ptid, int verbose)
       lp->stopped = 1;
     }
 
-  if (verbose)
-    printf_filtered (_("[New %s]\n"), target_pid_to_str (ptid));
-
   return 0;
 }
 
@@ -2089,8 +2086,6 @@ retry:
 		}
 
 	      add_thread (lp->ptid);
-	      printf_unfiltered (_("[New %s]\n"),
-				 target_pid_to_str (lp->ptid));
 	    }
 
 	  /* Save the trap's siginfo in case we need it later.  */
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index d2398df..16c223b 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -710,9 +710,6 @@ attach_thread (ptid_t ptid, const td_thrhandle_t *th_p,
   tp->private = xmalloc (sizeof (struct private_thread_info));
   memset (tp->private, 0, sizeof (struct private_thread_info));
 
-  if (verbose)
-    printf_unfiltered (_("[New %s]\n"), target_pid_to_str (ptid));
-
   /* Enable thread event reporting for this thread.  */
   err = td_thr_event_enable_p (th_p, 1);
   if (err != TD_OK)
diff --git a/gdb/procfs.c b/gdb/procfs.c
index e288459..2cac76a 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -4096,11 +4096,8 @@ wait_again:
 		    temp_ptid = MERGEPID (pi->pid, temp_tid);
 		    /* If not in GDB's thread list, add it.  */
 		    if (!in_thread_list (temp_ptid))
-		      {
-			printf_filtered (_("[New %s]\n"),
-					 target_pid_to_str (temp_ptid));
-			add_thread (temp_ptid);
-		      }
+		      add_thread (temp_ptid);
+
 		    /* Return to WFI, but tell it to immediately resume. */
 		    status->kind = TARGET_WAITKIND_SPURIOUS;
 		    return inferior_ptid;
@@ -4166,11 +4163,7 @@ wait_again:
 		    /* If not in GDB's thread list, add it.  */
 		    temp_ptid = MERGEPID (pi->pid, temp_tid);
 		    if (!in_thread_list (temp_ptid))
-		      {
-			printf_filtered (_("[New %s]\n"),
-					 target_pid_to_str (temp_ptid));
-			add_thread (temp_ptid);
-		      }
+		      add_thread (temp_ptid);
 
 		    status->kind = TARGET_WAITKIND_STOPPED;
 		    status->value.sig = 0;
@@ -4257,7 +4250,6 @@ wait_again:
 		   * If we don't create a procinfo, resume may be unhappy
 		   * later.
 		   */
-		  printf_filtered (_("[New %s]\n"), target_pid_to_str (retval));
 		  add_thread (retval);
 		  if (find_procinfo (PIDGET (retval), TIDGET (retval)) == NULL)
 		    create_procinfo (PIDGET (retval), TIDGET (retval));
diff --git a/gdb/remote.c b/gdb/remote.c
index 452af07..69faedb 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1049,12 +1049,7 @@ record_currthread (int currthread)
   /* If this is a new thread, add it to GDB's thread list.
      If we leave it up to WFI to do this, bad things will happen.  */
   if (!in_thread_list (pid_to_ptid (currthread)))
-    {
-      add_thread (pid_to_ptid (currthread));
-      ui_out_text (uiout, "[New ");
-      ui_out_text (uiout, target_pid_to_str (pid_to_ptid (currthread)));
-      ui_out_text (uiout, "]\n");
-    }
+    add_thread (pid_to_ptid (currthread));
 }
 
 static char *last_pass_packet;
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index bf40696..c139397 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -461,10 +461,7 @@ sol_thread_wait (ptid_t ptid, struct target_waitstatus *ourstatus)
       if (is_thread (rtnval)
 	  && !ptid_equal (rtnval, save_ptid)
 	  && !in_thread_list (rtnval))
-	{
-	  printf_filtered ("[New %s]\n", target_pid_to_str (rtnval));
-	  add_thread (rtnval);
-	}
+	add_thread (rtnval);
     }
 
   /* During process initialization, we may get here without the thread
diff --git a/gdb/thread.c b/gdb/thread.c
index 3c8644b..86b919d 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -116,11 +116,8 @@ init_thread_list (void)
   thread_list = NULL;
 }
 
-/* add_thread now returns a pointer to the new thread_info, 
-   so that back_ends can initialize their private data.  */
-
 struct thread_info *
-add_thread (ptid_t ptid)
+add_thread_silent (ptid_t ptid)
 {
   struct thread_info *tp;
 
@@ -130,9 +127,20 @@ add_thread (ptid_t ptid)
   tp->num = ++highest_thread_num;
   tp->next = thread_list;
   thread_list = tp;
+
+  printf_filtered (_("[New %s]\n"), target_pid_to_str (ptid));
+
   return tp;
 }
 
+struct thread_info *
+add_thread (ptid_t ptid)
+{
+  printf_filtered (_("[New %s]\n"), target_pid_to_str (ptid));
+
+  return add_thread_silent (ptid);
+}
+
 void
 delete_thread (ptid_t ptid)
 {
diff --git a/gdb/win32-nat.c b/gdb/win32-nat.c
index 07ebef9..9818839 100644
--- a/gdb/win32-nat.c
+++ b/gdb/win32-nat.c
@@ -1317,10 +1317,6 @@ get_win32_debug_event (int pid, struct target_waitstatus *ourstatus)
       /* Record the existence of this thread */
       th = win32_add_thread (current_event.dwThreadId,
 			     current_event.u.CreateThread.hThread);
-      if (info_verbose)
-	printf_unfiltered ("[New %s]\n",
-			   target_pid_to_str (
-			     pid_to_ptid (current_event.dwThreadId)));
       retval = current_event.dwThreadId;
       break;
 
-- 
1.5.3.5

Re: [RFA] Inform about new thread in a single place.

[Daniel Jacobowitz]

On Fri, Dec 14, 2007 at 10:40:20PM +0300, Vladimir Prus wrote:
> Here's the revised patch. OK?

linux-nat.c, linux-thread-db.c, and win32-nat.c all print only under
control of a verbose flag.  While we're unifying the code we should
unify the behavior too - should this always be shown, or not?

The call site in linux-nat.c never received the verbose flag, so it
has not been printing for a while.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Inform about new thread in a single place.

[Vladimir Prus]

On Friday 14 December 2007 22:47:21 Daniel Jacobowitz wrote:
> On Fri, Dec 14, 2007 at 10:40:20PM +0300, Vladimir Prus wrote:
> > Here's the revised patch. OK?
> 
> linux-nat.c, linux-thread-db.c, and win32-nat.c all print only under
> control of a verbose flag.  While we're unifying the code we should
> unify the behavior too - should this always be shown, or not?

I think it should always be shown.  I find it hard to believe
that user want to know about new thread on one platform, and don't
want on another platform.

> The call site in linux-nat.c never received the verbose flag, so it
> has not been printing for a while.

Right. That's why the patch removes the print there. The ChangeLog
entry I've posted had it backward, but in reality lin_lwp_attach_lwp
is called from linux-thread-db.c:attach_thread -- which did the printing.

Now that you mention it, it appears the 'verbose' parameter can be just removed.

- Volodya


 

Re: [RFA] Inform about new thread in a single place.

[Eli Zaretskii]

> Date: Fri, 14 Dec 2007 14:47:21 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Mark Kettenis <mark.kettenis@xs4all.nl>, gdb-patches@sources.redhat.com
> 
> On Fri, Dec 14, 2007 at 10:40:20PM +0300, Vladimir Prus wrote:
> > Here's the revised patch. OK?
> 
> linux-nat.c, linux-thread-db.c, and win32-nat.c all print only under
> control of a verbose flag.  While we're unifying the code we should
> unify the behavior too - should this always be shown, or not?

Please also amend the manual as appropriate (it now says the message
is shown unconditionally).

Re: [RFA] Inform about new thread in a single place.

[Mark Kettenis]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Fri, 14 Dec 2007 22:40:20 +0300
> 
> > > What about me adding 'add_thread_silent' function that will
> > > not print any message, and using it in bds-uthread.c?
> > 
> > That'd be acceptable to me.  I'm not absolutely true that none of the
> > modules you mention above don't need this, but we can fix those when
> > we notice the problem.
> 
> Here's the revised patch. OK?

I don't think so.  Look at the output from debugging a threaded
program and ask yourself if you really have that many new threads ;)

Also, this makes some platforms now print the message, where it didn't
before (for example on win32).  Please make sure you get approval from
the relevant maintainers for any changed behaviour.

Cheers,

Mark

> 	* thread.c (add_thread_silent): Renamed
> 	from add_thread.
> 	(add_thread): Announce new thread.
> 	* gdbthread.h (add_thread_silent): Declare.
> 	* inf-ttrace.c (inf_ttrace_wait): Don't
> 	inform about new thread, as add_thread is always
> 	called too, and will take care of that.
> 	* infrun.c (handle_inferior_event): Likewise.
> 	* procfs.c (procfs_wait): Likewise.
> 	* remote.c (remote_currthread): Likewise.
> 	* sol-thread.c (sol_thread_wait): Likewise.
> 	* win32-nat.c (get_win32_debug_event): Likewise.
> 	* gdb/linux-nat.c (lin_lwp_attach_lwp): Likewise.
> 	* linux-thread-db.c (attach_thread): Don't inform
> 	about new thread.  This is called only from
> 	linux-thread-db.c:attach_thread, which will take care.

Re: [RFA] Inform about new thread in a single place.

[Vladimir Prus]

On Friday 14 December 2007 22:54:28 you wrote:
> > From: Vladimir Prus <vladimir@codesourcery.com>
> > Date: Fri, 14 Dec 2007 22:40:20 +0300
> > 
> > > > What about me adding 'add_thread_silent' function that will
> > > > not print any message, and using it in bds-uthread.c?
> > > 
> > > That'd be acceptable to me.  I'm not absolutely true that none of the
> > > modules you mention above don't need this, but we can fix those when
> > > we notice the problem.
> > 
> > Here's the revised patch. OK?
> 
> I don't think so.  Look at the output from debugging a threaded
> program and ask yourself if you really have that many new threads ;)

Doh! Of course, that was so small tweak that testing it was not necessary ;-)
Here's yet another revision.

> Also, this makes some platforms now print the message, where it didn't
> before (for example on win32).  Please make sure you get approval from
> the relevant maintainers for any changed behaviour.

Hmm, this patch actually makes gdb behave consistently for all platforms,
so it's probably global question, as opposed to specific detail of any given
platform behaviour.

- Volodya

	* thread.c (add_thread_silent): Renamed
	from add_thread.
	(add_thread): Announce new thread.
	* gdbthread.h (add_thread_silent): Declare.
	* inf-ttrace.c (inf_ttrace_wait): Don't
	inform about new thread, as add_thread is always
	called too, and will take care of that.
	* infrun.c (handle_inferior_event): Likewise.
	* procfs.c (procfs_wait): Likewise.
	* remote.c (remote_currthread): Likewise.
	* sol-thread.c (sol_thread_wait): Likewise.
	* win32-nat.c (get_win32_debug_event): Likewise.
	* linux-thread-db.c (attach_thread): Likewise.
	* linux-nat.c (lin_lwp_attach_lwp): Don't inform
	about new thread.  This is called only from
	linux-thread-db.c:attach_thread, which will take care.
	Remove the verbose parameter.
	* linux-nat.h (lin_lwp_attach_lwp): Adjust prototype.
---
 gdb/bsd-uthread.c     |    2 +-
 gdb/gdbthread.h       |   11 ++++++++---
 gdb/inf-ttrace.c      |    1 -
 gdb/infrun.c          |    8 +-------
 gdb/linux-nat.c       |    7 +------
 gdb/linux-nat.h       |    2 +-
 gdb/linux-thread-db.c |    5 +----
 gdb/procfs.c          |   14 +++-----------
 gdb/remote.c          |    7 +------
 gdb/sol-thread.c      |    5 +----
 gdb/thread.c          |   15 +++++++++++----
 gdb/win32-nat.c       |    4 ----
 12 files changed, 29 insertions(+), 52 deletions(-)

diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
index 3c8714a..064b7de 100644
--- a/gdb/bsd-uthread.c
+++ b/gdb/bsd-uthread.c
@@ -366,7 +366,7 @@ bsd_uthread_wait (ptid_t ptid, struct target_waitstatus *status)
   if (ptid_get_tid (ptid) != 0 && !in_thread_list (ptid)
       && ptid_get_tid (inferior_ptid) == 0)
     {
-      add_thread (ptid);
+      add_thread_silent (ptid);
       inferior_ptid = ptid;
     }
 
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 12e0bcc..403e7e3 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -69,11 +69,16 @@ struct thread_info
 /* Create an empty thread list, or empty the existing one.  */
 extern void init_thread_list (void);
 
-/* Add a thread to the thread list.
-   Note that add_thread now returns the handle of the new thread,
-   so that the caller may initialize the private thread data.  */
+/* Add a thread to the thread list, print a message
+   that a new thread is found, and return the pointer to
+   the new thread.  Caller my use this pointer to 
+   initialize the private thread data.  */
 extern struct thread_info *add_thread (ptid_t ptid);
 
+/* Same as add_thread, but does not print a message
+   about new thread.  */
+extern struct thread_info *add_thread_silent (ptid_t ptid);
+
 /* Delete an existing thread list entry.  */
 extern void delete_thread (ptid_t);
 
diff --git a/gdb/inf-ttrace.c b/gdb/inf-ttrace.c
index 420133a..ffbfe9c 100644
--- a/gdb/inf-ttrace.c
+++ b/gdb/inf-ttrace.c
@@ -962,7 +962,6 @@ inf_ttrace_wait (ptid_t ptid, struct target_waitstatus *ourstatus)
 		  sizeof (struct inf_ttrace_private_thread_info));
 	  inf_ttrace_num_lwps++;
 	}
-      printf_filtered (_("[New %s]\n"), target_pid_to_str (ptid));
       ti = add_thread (ptid);
       ti->private =
 	xmalloc (sizeof (struct inf_ttrace_private_thread_info));
diff --git a/gdb/infrun.c b/gdb/infrun.c
index e651425..1cd5e98 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1339,13 +1339,7 @@ handle_inferior_event (struct execution_control_state *ecs)
 
   if (ecs->ws.kind != TARGET_WAITKIND_EXITED
       && ecs->ws.kind != TARGET_WAITKIND_SIGNALLED && ecs->new_thread_event)
-    {
-      add_thread (ecs->ptid);
-
-      ui_out_text (uiout, "[New ");
-      ui_out_text (uiout, target_pid_to_str (ecs->ptid));
-      ui_out_text (uiout, "]\n");
-    }
+    add_thread (ecs->ptid);
 
   switch (ecs->ws.kind)
     {
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 9a39ab6..6466bf0 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -870,7 +870,7 @@ exit_lwp (struct lwp_info *lp)
    be attached.  */
 
 int
-lin_lwp_attach_lwp (ptid_t ptid, int verbose)
+lin_lwp_attach_lwp (ptid_t ptid)
 {
   struct lwp_info *lp;
 
@@ -955,9 +955,6 @@ lin_lwp_attach_lwp (ptid_t ptid, int verbose)
       lp->stopped = 1;
     }
 
-  if (verbose)
-    printf_filtered (_("[New %s]\n"), target_pid_to_str (ptid));
-
   return 0;
 }
 
@@ -2089,8 +2086,6 @@ retry:
 		}
 
 	      add_thread (lp->ptid);
-	      printf_unfiltered (_("[New %s]\n"),
-				 target_pid_to_str (lp->ptid));
 	    }
 
 	  /* Save the trap's siginfo in case we need it later.  */
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 43686cf..9b8d3bf 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -95,7 +95,7 @@ void linux_proc_pending_signals (int pid, sigset_t *pending, sigset_t *blocked,
 /* linux-nat functions for handling fork events.  */
 extern void linux_enable_event_reporting (ptid_t ptid);
 
-extern int lin_lwp_attach_lwp (ptid_t ptid, int verbose);
+extern int lin_lwp_attach_lwp (ptid_t ptid);
 
 /* Iterator function for lin-lwp's lwp list.  */
 struct lwp_info *iterate_over_lwps (int (*callback) (struct lwp_info *, 
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index d2398df..41b0166 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -702,7 +702,7 @@ attach_thread (ptid_t ptid, const td_thrhandle_t *th_p,
     return;			/* A zombie thread -- do not attach.  */
 
   /* Under GNU/Linux, we have to attach to each and every thread.  */
-  if (lin_lwp_attach_lwp (BUILD_LWP (ti_p->ti_lid, GET_PID (ptid)), 0) < 0)
+  if (lin_lwp_attach_lwp (BUILD_LWP (ti_p->ti_lid, GET_PID (ptid))) < 0)
     return;
 
   /* Add the thread to GDB's thread list.  */
@@ -710,9 +710,6 @@ attach_thread (ptid_t ptid, const td_thrhandle_t *th_p,
   tp->private = xmalloc (sizeof (struct private_thread_info));
   memset (tp->private, 0, sizeof (struct private_thread_info));
 
-  if (verbose)
-    printf_unfiltered (_("[New %s]\n"), target_pid_to_str (ptid));
-
   /* Enable thread event reporting for this thread.  */
   err = td_thr_event_enable_p (th_p, 1);
   if (err != TD_OK)
diff --git a/gdb/procfs.c b/gdb/procfs.c
index e288459..2cac76a 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -4096,11 +4096,8 @@ wait_again:
 		    temp_ptid = MERGEPID (pi->pid, temp_tid);
 		    /* If not in GDB's thread list, add it.  */
 		    if (!in_thread_list (temp_ptid))
-		      {
-			printf_filtered (_("[New %s]\n"),
-					 target_pid_to_str (temp_ptid));
-			add_thread (temp_ptid);
-		      }
+		      add_thread (temp_ptid);
+
 		    /* Return to WFI, but tell it to immediately resume. */
 		    status->kind = TARGET_WAITKIND_SPURIOUS;
 		    return inferior_ptid;
@@ -4166,11 +4163,7 @@ wait_again:
 		    /* If not in GDB's thread list, add it.  */
 		    temp_ptid = MERGEPID (pi->pid, temp_tid);
 		    if (!in_thread_list (temp_ptid))
-		      {
-			printf_filtered (_("[New %s]\n"),
-					 target_pid_to_str (temp_ptid));
-			add_thread (temp_ptid);
-		      }
+		      add_thread (temp_ptid);
 
 		    status->kind = TARGET_WAITKIND_STOPPED;
 		    status->value.sig = 0;
@@ -4257,7 +4250,6 @@ wait_again:
 		   * If we don't create a procinfo, resume may be unhappy
 		   * later.
 		   */
-		  printf_filtered (_("[New %s]\n"), target_pid_to_str (retval));
 		  add_thread (retval);
 		  if (find_procinfo (PIDGET (retval), TIDGET (retval)) == NULL)
 		    create_procinfo (PIDGET (retval), TIDGET (retval));
diff --git a/gdb/remote.c b/gdb/remote.c
index 452af07..69faedb 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1049,12 +1049,7 @@ record_currthread (int currthread)
   /* If this is a new thread, add it to GDB's thread list.
      If we leave it up to WFI to do this, bad things will happen.  */
   if (!in_thread_list (pid_to_ptid (currthread)))
-    {
-      add_thread (pid_to_ptid (currthread));
-      ui_out_text (uiout, "[New ");
-      ui_out_text (uiout, target_pid_to_str (pid_to_ptid (currthread)));
-      ui_out_text (uiout, "]\n");
-    }
+    add_thread (pid_to_ptid (currthread));
 }
 
 static char *last_pass_packet;
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index bf40696..c139397 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -461,10 +461,7 @@ sol_thread_wait (ptid_t ptid, struct target_waitstatus *ourstatus)
       if (is_thread (rtnval)
 	  && !ptid_equal (rtnval, save_ptid)
 	  && !in_thread_list (rtnval))
-	{
-	  printf_filtered ("[New %s]\n", target_pid_to_str (rtnval));
-	  add_thread (rtnval);
-	}
+	add_thread (rtnval);
     }
 
   /* During process initialization, we may get here without the thread
diff --git a/gdb/thread.c b/gdb/thread.c
index 3c8644b..33c3f25 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -116,11 +116,8 @@ init_thread_list (void)
   thread_list = NULL;
 }
 
-/* add_thread now returns a pointer to the new thread_info, 
-   so that back_ends can initialize their private data.  */
-
 struct thread_info *
-add_thread (ptid_t ptid)
+add_thread_silent (ptid_t ptid)
 {
   struct thread_info *tp;
 
@@ -133,6 +130,16 @@ add_thread (ptid_t ptid)
   return tp;
 }
 
+struct thread_info *
+add_thread (ptid_t ptid)
+{
+  struct thread_info *result = add_thread_silent (ptid);
+
+  printf_filtered (_("[New %s]\n"), target_pid_to_str (ptid));
+  
+  return result;
+}
+
 void
 delete_thread (ptid_t ptid)
 {
diff --git a/gdb/win32-nat.c b/gdb/win32-nat.c
index 07ebef9..9818839 100644
--- a/gdb/win32-nat.c
+++ b/gdb/win32-nat.c
@@ -1317,10 +1317,6 @@ get_win32_debug_event (int pid, struct target_waitstatus *ourstatus)
       /* Record the existence of this thread */
       th = win32_add_thread (current_event.dwThreadId,
 			     current_event.u.CreateThread.hThread);
-      if (info_verbose)
-	printf_unfiltered ("[New %s]\n",
-			   target_pid_to_str (
-			     pid_to_ptid (current_event.dwThreadId)));
       retval = current_event.dwThreadId;
       break;
 
-- 
1.5.3.5