From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3075 invoked by alias); 24 Jul 2009 14:16:59 -0000 Received: (qmail 2848 invoked by uid 22791); 24 Jul 2009 14:16:57 -0000 X-SWARE-Spam-Status: No, hits=-0.7 required=5.0 tests=AWL,BAYES_50,J_CHICKENPOX_63,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 24 Jul 2009 14:16:51 +0000 Received: (qmail 23884 invoked from network); 24 Jul 2009 14:16:49 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 24 Jul 2009 14:16:49 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: nto_extra_thread_info Date: Fri, 24 Jul 2009 16:47:00 -0000 User-Agent: KMail/1.9.10 Cc: Aleksandar Ristovski References: In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200907241516.48134.pedro@codesourcery.com> X-IsSubscribed: yes 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 X-SW-Source: 2009-07/txt/msg00597.txt.bz2 On Friday 12 June 2009 19:57:02, Aleksandar Ristovski wrote: > > This patch introduces extra thread info for nto target, and > fixes a bug in procfs_find_new_threads. > Index: gdb/nto-procfs.c > =================================================================== > --- gdb/nto-procfs.c (revision 2) > +++ gdb/nto-procfs.c (working copy) > @@ -220,11 +220,94 @@ static int > procfs_thread_alive (struct target_ops *ops, ptid_t ptid) > { > + > + status.tid = tid; > + if ((err = devctl (ctl_fd, DCMD_PROC_TIDSTATUS, > + &status, sizeof (status), 0)) != EOK) > + return 0; > + > + /* Thread is alive or dead but not yet joined, > + or dead and there is an alive (or dead unjoined) thread with > + higher tid. We return tidinfo. ^ Missing space. This "We return tidinfo." looks out of place though. I suspect this whole comment was copied from the devctl's docs or implementation. > + > + Client should check if the tid is the same as > + requested; if not, requested tid is dead. */ > + return (status.tid == tid) && (status.state != STATE_DEAD); > +} > + > +static void > +update_thread_private_data_name (struct thread_info *new_thread, > + const char *newname) > +{ > + > + if (!new_thread->private) > + { > + new_thread->private = xmalloc (sizeof (struct private_thread_info) > + + newnamelen + 1); Note: pedanticaly, '+ 1' here isn't really needed, since the name field was declared as name[1]. FYI, a standard practice is to use offsetof(struct foo, flexiblearraymember) + arraysize instead of sizeof. > +static void > +update_thread_private_data (struct thread_info *new_thread, > + pthread_t tid, int state, int flags) > +{ > + struct private_thread_info *pti; > + procfs_info pidinfo; > + struct _thread_name *tn; > + procfs_threadctl tctl; > +#if _NTO_VERSION > 630 > + gdb_assert (new_thread != NULL); > + > + if (devctl (ctl_fd, DCMD_PROC_INFO, &pidinfo, > + sizeof(pidinfo), 0) != EOK) > + return; Should this set the private data to a known state, or release it, so it doesn't show stale data? (can this really happen?) > 2009-06-12 Aleksandar Ristovski > > * nto-tdep.c (nto_thread_state_str): New array. > (nto_extra_thread_info): New function definition. > * nto-tdep.h (gdbthread.h): New include. > (private_thread_info): New struct. > (nto_extra_thread_info): New declaration. > * nto-procfs.c (procfs_thread_alive): Properly check if > thread is still alive. > (update_thread_private_data_name, update_thread_private_data): New > function definition. > (procfs_find_new_threads): Fetch thread private data. > (init_procfs_ops): Register to_extra_thread_info. Otherwise, looks okay to me. -- Pedro Alves