This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: nto_extra_thread_info


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  <aristovski@qnx.com>
> 
>         * 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]