This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/4] Create private_thread_info hierarchy
On 2017-11-23 09:41, Pedro Alves wrote:
On 11/22/2017 04:41 PM, Simon Marchi wrote:
From: Simon Marchi <simon.marchi@polymtl.ca>
Including gdbthread.h from darwin-nat.h gave these errors:
/Users/simark/src/binutils-gdb/gdb/gdbthread.h:609:3: error: must use
'class' tag to refer to type 'thread_info' in this scope
thread_info *m_thread;
^
class
/usr/include/mach/thread_act.h:240:15: note: class 'thread_info' is
hidden by a non-type declaration of 'thread_info' here
kern_return_t thread_info
^
It turns out that there is a thread_info function in the
Darwin/XNU/mach API:
http://web.mit.edu/darwin/src/modules/xnu/osfmk/man/thread_info.html
Therefore, I had to add the class keyword at a couple of places in
gdbthread.h,
I don't really see a way around it.
The way around it is to move all of gdb under "namespace gdb", and then
references to thread_info hit gdb::thread_info instead. :-)
But that would require us to improve gdb's C++ debugging support, so
that we don't have to type gdb:: every time! Is such thing even
possible?!
private:
- thread_info *m_thread;
+ /* Use the "class" keyword here, because of a class with a
"thread_info"
+ function in the Darwin API. */
+ class thread_info *m_thread;
This comment sounds incorrect. A a "thread_info" function/method
inside some class couldn't possibly interfere, right? The issue is
that there's a _free-function_ called thread_info, right?
Err yeah, the "class with a" part should not be there:
+ /* Use the "class" keyword here, because of a "thread_info"
+ function in the Darwin API. */
Some nits below.
@@ -776,8 +779,10 @@ sync_threadlists (void)
if (cmp_result == 0)
{
- gbuf[gi]->priv->pdtid = pdtid;
- gbuf[gi]->priv->tid = tid;
+ aix_thread_info *priv = (aix_thread_info *) gbuf[gi]->priv.get
();
...
@@ -808,8 +815,9 @@ static int
iter_tid (struct thread_info *thread, void *tidp)
{
const pthdb_tid_t tid = *(pthdb_tid_t *)tidp;
+ aix_thread_info *priv = (aix_thread_info *) thread->priv.get ();
This seems to scream for a "get_aix_thread_info (thread_info *);"
function.
Will do.
- return (thread->priv->tid == tid);
+ return priv->tid == tid;
}
@@ -1381,11 +1379,11 @@ thread_db_pid_to_str (struct target_ops *ops,
ptid_t ptid)
if (thread_info != NULL && thread_info->priv != NULL)
{
static char buf[64];
- thread_t tid;
+ thread_db_thread_info *priv
+ = (thread_db_thread_info *) thread_info->priv.get ();
...
- if (info->priv->dying)
+ thread_db_thread_info *priv = (thread_db_thread_info *)
info->priv.get ();
+
+ if (priv->dying)
return "Exiting";
...
return NULL;
@@ -1434,7 +1434,9 @@ thread_db_thread_handle_to_thread_info (struct
target_ops *ops,
ALL_NON_EXITED_THREADS (tp)
{
- if (tp->inf == inf && tp->priv != NULL && handle_tid ==
tp->priv->tid)
+ thread_db_thread_info *priv = (thread_db_thread_info *)
tp->priv.get ();
You know what I'll say, right? :-)
I think I'm starting to get the idea.
+
+ if (tp->inf == inf && priv != NULL && handle_tid == priv->tid)
return tp;
}
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 1da1a98..5906eb6 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -248,38 +248,24 @@ static void
update_thread_private_data_name (struct thread_info *new_thread,
const char *newname)
{
- int newnamelen;
- struct private_thread_info *pti;
+ nto_thread_info *pti = (nto_thread_info *) new_thread->priv.get ();
The comment really applies to all ports.
Hehehe.
-/* Private data that we'll store in (struct thread_info)->private.
*/
-struct private_thread_info
+/* Private data that we'll store in (struct thread_info)->priv. */
+struct remote_thread_info : public private_thread_info
{
- char *extra;
- char *name;
- int core;
+ std::string extra;
+ std::string name;
+ int core = -1;
/* Thread handle, perhaps a pthread_t or thread_t value, stored as
a
sequence of bytes. */
- gdb::byte_vector *thread_handle;
+ gdb::byte_vector thread_handle;
Ah, OK, you did the pointer->value thing here. Great. :-)
Yes, but as I mentioned in my other reply, I think it's better to do it
in the previous patch.
Thanks,
Simon