This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: gdb bug 12417
- From: Keith Seitz <keiths at redhat dot com>
- To: Mohsan Saleem <mohsansaleem_ms at yahoo dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Wed, 19 Sep 2012 06:43:57 -0700
- Subject: Re: gdb bug 12417
- References: <A62F3BCAE6F6B540A2F2C0E7E4387B9505B5AA72@EU-MBX-01.mgc.mentorg.com> <1347974206.13036.YahooMailNeo@web114403.mail.gq1.yahoo.com>
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