From 75acb4867dc8bdd701983af6899d823c9e2e53a4 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Fri, 29 Jun 2018 20:45:35 +0100 Subject: [PATCH] Improve alignment of "info threads" output, align "Target Id" column It's long annoyed me that "info threads"'s columns are misaligned. Particularly the "Target Id" column's content is usually longer than the specified column width, so the table ends up with the "Frame" column misaligned. For example, currently we get this: (gdb) info threads Id Target Id Frame 1 Thread 0x7ffff7fb5740 (LWP 9056) "threads" 0x00007ffff7bc28ad in __pthread_join (threadid=140737345763072, thread_return=0x7fffffffd3e8) at pthread_join.c:90 2 Thread 0x7ffff7803700 (LWP 9060) "function0" thread_function0 (arg=0x0) at threads.c:90 * 3 Thread 0x7ffff7002700 (LWP 9061) "threads" thread_function1 (arg=0x1) at threads.c:106 The fact that the "Frame" heading is in a weird spot is particularly annoying. This commit turns the above into into this: (gdb) info threads Id Target Id Frame 1 Thread 0x7ffff7fb5740 (LWP 7548) "threads" 0x00007ffff7bc28ad in __pthread_join (threadid=140737345763072, thread_return=0x7fffffffd3e8) at pthread_join.c:90 2 Thread 0x7ffff7803700 (LWP 7555) "function0" thread_function0 (arg=0x0) at threads.c:91 * 3 Thread 0x7ffff7002700 (LWP 7557) "threads" thread_function1 (arg=0x1) at threads.c:104 It does that by computing the max width of the "Target Id" column and using that as column width when creating the table. This results in calling target_pid_to_str / target_extra_thread_info / target_thread_name twice for each thread, but I think that it doesn't matter in practice performance-wise, because the remote target caches the info, and with native targets it shouldn't be noticeable. It could matter if we have many threads (say, thousands), but then "info threads" is practically useless in such a scenario anyway -- better thread filtering and aggregation would be necessary. (Note: I have an old branch somewhere where I attempted at making gdb's "info threads"-like tables follow a model/view design, so that a general framework took care of issues like these, but it's incomplete and a much bigger change. This patch doesn't prevent going in that direction in the future, of course.) gdb/ChangeLog: 2018-06-29 Pedro Alves * thread.c (thread_target_id_str): New, factored out from ... (print_thread_info_1): ... here. Use it to compute the max "Target Id" column width. gdb/testsuite/ChangeLog: 2018-06-29 Pedro Alves * gdb.threads/names.exp: Adjust expected "info threads" output. --- gdb/ChangeLog | 6 +++ gdb/testsuite/ChangeLog | 4 ++ gdb/testsuite/gdb.threads/names.exp | 2 +- gdb/thread.c | 65 +++++++++++++++++++---------- 4 files changed, 54 insertions(+), 23 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 2d75bd2ac15..0cbccf5f8a6 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2018-06-29 Pedro Alves + + * thread.c (thread_target_id_str): New, factored out from ... + (print_thread_info_1): ... here. Use it to compute the max + "Target Id" column width. + 2018-06-29 Pedro Alves * remote.c (remote_target::extra_thread_info): Delete diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 8a110719934..93c849c040c 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2018-06-29 Pedro Alves + + * gdb.threads/names.exp: Adjust expected "info threads" output. + 2018-06-29 Pedro Alves * gdb.opt/inline-break.exp (line number, address): Add "info diff --git a/gdb/testsuite/gdb.threads/names.exp b/gdb/testsuite/gdb.threads/names.exp index 44e21e65e99..fbc10f5506a 100644 --- a/gdb/testsuite/gdb.threads/names.exp +++ b/gdb/testsuite/gdb.threads/names.exp @@ -31,7 +31,7 @@ if ![runto "all_threads_ready"] { } gdb_test "info threads" \ - [multi_line "\\* 1 .*\"main\" all_threads_ready.*" \ + [multi_line "\\* 1 .*\"main\"\[ \]\+all_threads_ready.*" \ " 2 .*\"carrot\".*" \ " 3 .*\"potato\".*" \ " 4 .*\"celery\".*" ] \ diff --git a/gdb/thread.c b/gdb/thread.c index 77b497e2812..fd92e78e064 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -1119,6 +1119,26 @@ should_print_thread (const char *requested_threads, int default_inf_num, return 1; } +/* Return the string to display in "info threads"'s "Target Id" + column, for TP. */ + +static std::string +thread_target_id_str (thread_info *tp) +{ + const char *target_id = target_pid_to_str (tp->ptid); + const char *extra_info = target_extra_thread_info (tp); + const char *name = tp->name != nullptr ? tp->name : target_thread_name (tp); + + if (extra_info != nullptr && name != nullptr) + return string_printf ("%s \"%s\" (%s)", target_id, name, extra_info); + else if (extra_info != nullptr) + return string_printf ("%s (%s)", target_id, extra_info); + else if (name != nullptr) + return string_printf ("%s \"%s\"", target_id, name); + else + return target_id; +} + /* Like print_thread_info, but in addition, GLOBAL_IDS indicates whether REQUESTED_THREADS is a list of global or per-inferior thread ids. */ @@ -1129,7 +1149,6 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads, int show_global_ids) { struct thread_info *tp; - const char *extra_info, *name, *target_id; struct inferior *inf; int default_inf_num = current_inferior ()->num; @@ -1155,6 +1174,9 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads, else { int n_threads = 0; + /* The width of the "Target Id" column. Grown below to + accommodate the largest entry. */ + size_t target_id_col_width = 17; ALL_THREADS (tp) { @@ -1162,6 +1184,13 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads, global_ids, pid, tp)) continue; + if (!uiout->is_mi_like_p ()) + { + target_id_col_width + = std::max (target_id_col_width, + thread_target_id_str (tp).size ()); + } + ++n_threads; } @@ -1182,7 +1211,8 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads, uiout->table_header (4, ui_left, "id-in-tg", "Id"); if (show_global_ids) uiout->table_header (4, ui_left, "id", "GId"); - uiout->table_header (17, ui_left, "target-id", "Target Id"); + uiout->table_header (target_id_col_width, ui_left, + "target-id", "Target Id"); uiout->table_header (1, ui_left, "frame", "Frame"); uiout->table_body (); } @@ -1224,33 +1254,24 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads, shared by several fields. For MI, we do the right thing instead. */ - target_id = target_pid_to_str (tp->ptid); - extra_info = target_extra_thread_info (tp); - name = tp->name ? tp->name : target_thread_name (tp); - if (uiout->is_mi_like_p ()) { - uiout->field_string ("target-id", target_id); - if (extra_info) + uiout->field_string ("target-id", target_pid_to_str (tp->ptid)); + + const char *extra_info = target_extra_thread_info (tp); + if (extra_info != nullptr) uiout->field_string ("details", extra_info); - if (name) + + const char *name = (tp->name != nullptr + ? tp->name + : target_thread_name (tp)); + if (name != NULL) uiout->field_string ("name", name); } else { - std::string contents; - - if (extra_info && name) - contents = string_printf ("%s \"%s\" (%s)", target_id, - name, extra_info); - else if (extra_info) - contents = string_printf ("%s (%s)", target_id, extra_info); - else if (name) - contents = string_printf ("%s \"%s\"", target_id, name); - else - contents = target_id; - - uiout->field_string ("target-id", contents.c_str ()); + uiout->field_string ("target-id", + thread_target_id_str (tp).c_str ()); } if (tp->state == THREAD_RUNNING) -- 2.30.2