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

Vladimir Prus
2007-12-14T20:45:26Z

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