Re: nto_extra_thread_info

Pedro Alves
2009-07-24T14:16:59Z

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