From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4079 invoked by alias); 19 Sep 2012 13:44:44 -0000 Received: (qmail 4066 invoked by uid 22791); 19 Sep 2012 13:44:43 -0000 X-SWARE-Spam-Status: No, hits=-7.1 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_CP X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 19 Sep 2012 13:44:26 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q8JDiQaG011867 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 19 Sep 2012 09:44:26 -0400 Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q8JDiN4p001961 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Wed, 19 Sep 2012 09:44:25 -0400 Message-ID: <5059CC1D.1040105@redhat.com> Date: Wed, 19 Sep 2012 13:44:00 -0000 From: Keith Seitz User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0 MIME-Version: 1.0 To: Mohsan Saleem CC: "gdb-patches@sourceware.org" Subject: Re: gdb bug 12417 References: <1347974206.13036.YahooMailNeo@web114403.mail.gq1.yahoo.com> In-Reply-To: <1347974206.13036.YahooMailNeo@web114403.mail.gq1.yahoo.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes 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: 2012-09/txt/msg00391.txt.bz2 On 09/18/2012 06:16 AM, Mohsan Saleem wrote: > Hi, > Attached patch is for bug 12417, for printing the name of threads while printing the information of a thread. > Thank you for you patch submission. I do not see your name listed in gdb/MAINTAINERS -- do you have an assignment? If not, let us know and we can help you get started with the paperwork. In general, I'd say the patch is definitely a feature we should consider adding. [Note: I am not a global maintainer.] > --- /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); target_thread_name can return NULL. There's an added space typo in "[New %s ]". > @@ -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 Likewise. > @@ -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); And again here. > @@ -1283,7 +1286,6 @@ > { > if (ptid_equal (inferior_ptid, null_ptid)) > error (_("No thread selected")); > - > if (target_has_stack) > { > if (is_exited (inferior_ptid)) This is a superfluous whitespace change. In general, we try to keep these types of changes in their own patches. It helps keep patches to the important changes. > @@ -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)) Same here. > @@ -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++; name could be NULL. > } > } > @@ -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, ")]"); And one more time with NULL from target_thread_name. This is also a change to the MI. It will definitely need the approval of an MI or global maintainer, as well as updated MI documentation and tests. inferior_command looks like it could benefit from similar treatment. I wonder if there are other places? Otherwise, running the test suite, this patch causes several regressions which will need fixing: 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 FAIL: gdb.threads/thread_events.exp: continue to threadfunc with messages enabled (saw 0, expected 1) It appears that these simply need to be massaged to account for the thread name output, although in one case, "(null)" is output for the thread name. Similar changes will need to be done for the manual as well (see, for example, section 4.10, "Debugging Programs with Multiple Threads," which contains several examples that could use updating). Keith