* [PATCH] gdb: change thread_info::name to unique_xmalloc_ptr, add helper function
@ 2021-09-03 2:29 Simon Marchi
2021-09-23 18:15 ` Tom Tromey
0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2021-09-03 2:29 UTC (permalink / raw)
To: gdb-patches
From: Simon Marchi <simon.marchi@polymtl.ca>
This started out as changing thread_info::name to a unique_xmalloc_ptr.
That showed that almost all users of that field had the same logic to
get a thread's name: use thread_info::name if non-nullptr, else ask the
target. Factor out this logic in a new thread_name free function. Make
the field private (rename to m_name) and add some accessors.
Change-Id: Iebdd95f4cd21fbefc505249bd1d05befc466a2fc
---
gdb/breakpoint.c | 3 +--
gdb/gdbthread.h | 30 ++++++++++++++++++++++++++----
gdb/infrun.c | 4 +---
gdb/python/py-infthread.c | 9 ++-------
gdb/thread.c | 36 +++++++++++++++++++++++-------------
5 files changed, 53 insertions(+), 29 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index f6c9683aecf..aab1c50acdc 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4565,13 +4565,12 @@ maybe_print_thread_hit_breakpoint (struct ui_out *uiout)
if (show_thread_that_caused_stop ())
{
- const char *name;
struct thread_info *thr = inferior_thread ();
uiout->text ("Thread ");
uiout->field_string ("thread-id", print_thread_id (thr));
- name = thr->name != NULL ? thr->name : target_thread_name (thr);
+ const char *name = thread_name (thr);
if (name != NULL)
{
uiout->text (" \"");
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 9c178f531d9..2990d8a9b28 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -232,7 +232,6 @@ class thread_info : public refcounted_object,
{
public:
explicit thread_info (inferior *inf, ptid_t ptid);
- ~thread_info ();
bool deletable () const;
@@ -283,9 +282,21 @@ class thread_info : public refcounted_object,
/* The inferior this thread belongs to. */
struct inferior *inf;
- /* The name of the thread, as specified by the user. This is NULL
- if the thread does not have a user-given name. */
- char *name = NULL;
+ /* The user-given name of the thread.
+
+ Returns nullptr if the thread does not have a user-given name. */
+ const char *name () const
+ {
+ return m_name.get ();
+ }
+
+ /* Set the user-given name of the thread.
+
+ Pass nullptr to clear the name. */
+ void set_name (gdb::unique_xmalloc_ptr<char> name)
+ {
+ m_name = std::move (name);
+ }
/* True means the thread is executing. Note: this is different
from saying that there is an active target and we are stopped at
@@ -491,6 +502,11 @@ class thread_info : public refcounted_object,
/* State of inferior thread to restore after GDB is done with an inferior
call. See `struct thread_suspend_state'. */
thread_suspend_state m_suspend;
+
+ /* The user-give name of the thread.
+
+ Nullptr if the thread does not have a user-given name. */
+ gdb::unique_xmalloc_ptr<char> m_name;
};
using thread_info_resumed_with_pending_wait_status_node
@@ -921,4 +937,10 @@ extern void print_selected_thread_frame (struct ui_out *uiout,
alive anymore. */
extern void thread_select (const char *tidstr, class thread_info *thr);
+/* Return THREAD's name.
+
+ If THREAD has a user-given name, return it. Otherwise, query the thread's
+ target to get the name. May return nullptr. */
+extern const char *thread_name (thread_info *thread);
+
#endif /* GDBTHREAD_H */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 694bbe665f4..877c3f02b66 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8186,12 +8186,10 @@ print_signal_received_reason (struct ui_out *uiout, enum gdb_signal siggnal)
;
else if (show_thread_that_caused_stop ())
{
- const char *name;
-
uiout->text ("\nThread ");
uiout->field_string ("thread-id", print_thread_id (thr));
- name = thr->name != NULL ? thr->name : target_thread_name (thr);
+ const char *name = thread_name (thr);
if (name != NULL)
{
uiout->text (" \"");
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 3315af5eb59..f43e175f62c 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -66,14 +66,10 @@ static PyObject *
thpy_get_name (PyObject *self, void *ignore)
{
thread_object *thread_obj = (thread_object *) self;
- const char *name;
THPY_REQUIRE_VALID (thread_obj);
- name = thread_obj->thread->name;
- if (name == NULL)
- name = target_thread_name (thread_obj->thread);
-
+ const char *name = thread_name (thread_obj->thread);
if (name == NULL)
Py_RETURN_NONE;
@@ -115,8 +111,7 @@ thpy_set_name (PyObject *self, PyObject *newvalue, void *ignore)
return -1;
}
- xfree (thread_obj->thread->name);
- thread_obj->thread->name = name.release ();
+ thread_obj->thread->set_name (std::move (name));
return 0;
}
diff --git a/gdb/thread.c b/gdb/thread.c
index a82fb49140a..f8e6a64eb1e 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -302,11 +302,6 @@ thread_info::thread_info (struct inferior *inf_, ptid_t ptid_)
this->m_suspend.waitstatus.kind = TARGET_WAITKIND_IGNORE;
}
-thread_info::~thread_info ()
-{
- xfree (this->name);
-}
-
/* See gdbthread.h. */
bool
@@ -1001,7 +996,7 @@ thread_target_id_str (thread_info *tp)
{
std::string 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);
+ const char *name = thread_name (tp);
if (extra_info != nullptr && name != nullptr)
return string_printf ("%s \"%s\" (%s)", target_id.c_str (), name,
@@ -1143,9 +1138,7 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads,
if (extra_info != nullptr)
uiout->field_string ("details", extra_info);
- const char *name = (tp->name != nullptr
- ? tp->name
- : target_thread_name (tp));
+ const char *name = thread_name (tp);
if (name != NULL)
uiout->field_string ("name", name);
}
@@ -1838,8 +1831,7 @@ thread_name_command (const char *arg, int from_tty)
arg = skip_spaces (arg);
info = inferior_thread ();
- xfree (info->name);
- info->name = arg ? xstrdup (arg) : NULL;
+ info->set_name (arg != nullptr ? make_unique_xstrdup (arg) : nullptr);
}
/* Find thread ids with a name, target pid, or extra info matching ARG. */
@@ -1866,10 +1858,10 @@ thread_find_command (const char *arg, int from_tty)
{
switch_to_inferior_no_thread (tp->inf);
- if (tp->name != NULL && re_exec (tp->name))
+ if (tp->name () != nullptr && re_exec (tp->name ()))
{
printf_filtered (_("Thread %s has name '%s'\n"),
- print_thread_id (tp), tp->name);
+ print_thread_id (tp), tp->name ());
match++;
}
@@ -2013,6 +2005,24 @@ update_thread_list (void)
update_threads_executing ();
}
+/* See gdbthread.h. */
+
+const char *
+thread_name (thread_info *thread)
+{
+ /* Use the manually set name if there is one. */
+ const char *name = thread->name ();
+ if (name != nullptr)
+ return name;
+
+ /* Otherwise, ask the target. Ensure we query the right target stack. */
+ scoped_restore_current_thread restore_thread;
+ if (thread->inf != current_inferior ())
+ switch_to_inferior_no_thread (thread->inf);
+
+ return target_thread_name (thread);
+}
+
/* Return a new value for the selected thread's id. Return a value of
0 if no thread is selected. If GLOBAL is true, return the thread's
global number. Otherwise return the per-inferior number. */
--
2.33.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gdb: change thread_info::name to unique_xmalloc_ptr, add helper function
2021-09-03 2:29 [PATCH] gdb: change thread_info::name to unique_xmalloc_ptr, add helper function Simon Marchi
@ 2021-09-23 18:15 ` Tom Tromey
2021-09-24 21:24 ` Simon Marchi
0 siblings, 1 reply; 3+ messages in thread
From: Tom Tromey @ 2021-09-23 18:15 UTC (permalink / raw)
To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
Simon> This started out as changing thread_info::name to a unique_xmalloc_ptr.
Simon> That showed that almost all users of that field had the same logic to
Simon> get a thread's name: use thread_info::name if non-nullptr, else ask the
Simon> target. Factor out this logic in a new thread_name free function. Make
Simon> the field private (rename to m_name) and add some accessors.
Thank you.
Simon> + /* The user-give name of the thread.
Missing an "n" on "given".
Simon> +/* Return THREAD's name.
Simon> +
Simon> + If THREAD has a user-given name, return it. Otherwise, query the thread's
Simon> + target to get the name. May return nullptr. */
Simon> +extern const char *thread_name (thread_info *thread);
It seems to me that this should probably be the only caller of
target_thread_name. So, what about putting the extra logic into
target_thread_name instead of having a new function?
One idea here may be to keep target_thread_name as a simple wrapper for
the target method. But in that case, I think target_thread_name's
declaration should be updated to say something like "don't call this,
use thread_name instead".
Other than this issue, this all looks great to me.
Tom
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gdb: change thread_info::name to unique_xmalloc_ptr, add helper function
2021-09-23 18:15 ` Tom Tromey
@ 2021-09-24 21:24 ` Simon Marchi
0 siblings, 0 replies; 3+ messages in thread
From: Simon Marchi @ 2021-09-24 21:24 UTC (permalink / raw)
To: Tom Tromey, Simon Marchi via Gdb-patches
On 2021-09-23 14:15, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Simon> This started out as changing thread_info::name to a unique_xmalloc_ptr.
> Simon> That showed that almost all users of that field had the same logic to
> Simon> get a thread's name: use thread_info::name if non-nullptr, else ask the
> Simon> target. Factor out this logic in a new thread_name free function. Make
> Simon> the field private (rename to m_name) and add some accessors.
>
> Thank you.
>
> Simon> + /* The user-give name of the thread.
>
> Missing an "n" on "given".
Fixed.
>
> Simon> +/* Return THREAD's name.
> Simon> +
> Simon> + If THREAD has a user-given name, return it. Otherwise, query the thread's
> Simon> + target to get the name. May return nullptr. */
> Simon> +extern const char *thread_name (thread_info *thread);
>
> It seems to me that this should probably be the only caller of
> target_thread_name. So, what about putting the extra logic into
> target_thread_name instead of having a new function?
>
> One idea here may be to keep target_thread_name as a simple wrapper for
> the target method. But in that case, I think target_thread_name's
> declaration should be updated to say something like "don't call this,
> use thread_name instead".
I prefer the latter, keeping target_thread_name a simple wrapper. I
added this to target_thread_name's doc:
You likely don't want to call this function, but use the thread_name
function instead, which prefers the user-given thread name, if set. */
> Other than this issue, this all looks great to me.
Thanks, I'll push the patch with those fixed.
Simon
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-09-24 21:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-03 2:29 [PATCH] gdb: change thread_info::name to unique_xmalloc_ptr, add helper function Simon Marchi
2021-09-23 18:15 ` Tom Tromey
2021-09-24 21:24 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).