From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27401 invoked by alias); 14 Dec 2007 19:42:07 -0000 Received: (qmail 27277 invoked by uid 22791); 14 Dec 2007 19:42:05 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 14 Dec 2007 19:42:01 +0000 Received: (qmail 32310 invoked from network); 14 Dec 2007 19:41:58 -0000 Received: from unknown (HELO wind.local) (vladimir@127.0.0.2) by mail.codesourcery.com with ESMTPA; 14 Dec 2007 19:41:58 -0000 From: Vladimir Prus To: Mark Kettenis Subject: Re: [RFA] Inform about new thread in a single place. Date: Fri, 14 Dec 2007 19:47:00 -0000 User-Agent: KMail/1.9.6 References: <200712142009.08263.vladimir@codesourcery.com> <200712142207.19754.vladimir@codesourcery.com> <200712141920.lBEJKVI1027649@brahms.sibelius.xs4all.nl> In-Reply-To: <200712141920.lBEJKVI1027649@brahms.sibelius.xs4all.nl> Cc: gdb-patches@sources.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200712142240.20597.vladimir@codesourcery.com> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-12/txt/msg00194.txt.bz2 On Friday 14 December 2007 22:20:31 you wrote: > > From: Vladimir Prus > > Date: Fri, 14 Dec 2007 22:07:19 +0300 > > > > On Friday 14 December 2007 21:47:45 Mark Kettenis wrote: > > > > From: Vladimir Prus > > > > 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