Re: [PATCH 01/22] [GDBserver] Multi-process + multi-arch: core + GNU/Linux x86*

[Yao Qi <yao@codesourcery.com>]

On 05/30/2013 08:13 PM, Pedro Alves wrote:

> diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
> index 6953d0e..39eb52e 100644
> --- a/gdb/gdbserver/inferiors.c
> +++ b/gdb/gdbserver/inferiors.c
> @@ -102,7 +102,6 @@ add_thread (ptid_t thread_id, void *target_data)
>       current_inferior = new_thread;
>
>     new_thread->target_data = target_data;
> -  set_inferior_regcache_data (new_thread, new_register_cache ());

After read your introduction of this patch, it looks right to me. 
However, we may need some comments on creating regcache lazily in the 
code.  One paragraph in the mail is pretty good,

> The threads' regcaches are now created lazilly.  The old scheme where
> we created each of them when we added a new thread doesn't work
> anymore, because we add the main thread/lwp before we see it stop for
> the first time, and it is only when we see the thread stop for the
> first time that we have a chance of determining the inferior's
> architecture (through the_low_target.arch_setup).  Therefore when we
> add the main thread we don't know which architecture/tdesc its
> regcache should have.

I'd like to put this paragraph somewhere in the code, which will be helpful.

> diff --git a/gdb/gdbserver/regcache.h b/gdb/gdbserver/regcache.h
> index ce86322..700a2fe 100644
> --- a/gdb/gdbserver/regcache.h
> +++ b/gdb/gdbserver/regcache.h
> @@ -21,6 +21,7 @@
>
>   struct inferior_list_entry;
>   struct thread_info;
> +struct target_desc;
>
>   /* The register exists, it has a value, but we don't know what it is.
>      Used when inspecting traceframes.  */
> @@ -35,6 +36,9 @@ struct thread_info;
>
>   struct regcache
>   {
> +  /* The regcache's description.  */

"The regcache's target description" ?

-- 
Yao (齐尧)