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

Vladimir Prus
2007-12-14T19:42:07Z

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