From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29280 invoked by alias); 23 Nov 2017 16:54:01 -0000 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 Received: (qmail 29231 invoked by uid 89); 23 Nov 2017 16:53:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,KB_WAM_FROM_NAME_SINGLEWORD,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=time!, time X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 23 Nov 2017 16:53:58 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id vANGrpL8004733 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Thu, 23 Nov 2017 11:53:55 -0500 Received: by simark.ca (Postfix, from userid 112) id 1CD0C1E585; Thu, 23 Nov 2017 11:53:51 -0500 (EST) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 4D9C21E030; Thu, 23 Nov 2017 11:53:30 -0500 (EST) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Thu, 23 Nov 2017 16:54:00 -0000 From: Simon Marchi To: Pedro Alves Cc: Simon Marchi , gdb-patches@sourceware.org Subject: Re: [PATCH 3/4] Create private_thread_info hierarchy In-Reply-To: <845f4008-78ed-f188-0506-c67f9e20f531@redhat.com> References: <1511368867-19365-1-git-send-email-simon.marchi@ericsson.com> <1511368867-19365-4-git-send-email-simon.marchi@ericsson.com> <845f4008-78ed-f188-0506-c67f9e20f531@redhat.com> Message-ID: <6702102011e4d0ca966282ad8134a127@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.2 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Thu, 23 Nov 2017 16:53:51 +0000 X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg00562.txt.bz2 On 2017-11-23 09:41, Pedro Alves wrote: > On 11/22/2017 04:41 PM, Simon Marchi wrote: >> From: Simon Marchi > >> 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