RFC: don't call add_target for thread_db_ops

RFC: don't call add_target for thread_db_ops

[Tom Tromey]

Right now, "help target" will include this line:

    target multi-thread -- Threads and pthreads support

However, it doesn't make sense to invoke "target multi-thread".

This patch fixes the problem by not registering the multi-thread
target.  add_target does some needed initialization of the target_ops,
so I broke this out into a new function.

It isn't clear to me whether this patch requires a test case or not.
I'm not sure whether there are other unregistered targets; but if
there are, it seems unlikely that we test for their absence from the
help.

Built and regtested on x86-64 Fedora 18.

	* linux-thread-db.c (init_thread_db_ops): Call
	complete_target_initialization.
	(_initialize_thread_db): Don't call add_target.
	* target.c (complete_target_initialization): New function.
	(add_target_with_completer): Call it.
	* target.h (complete_target_initialization): Declare.
---
 gdb/linux-thread-db.c |  3 ++-
 gdb/target.c          | 23 ++++++++++++++++-------
 gdb/target.h          |  5 +++++
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 23c29c9..3696827 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -2057,6 +2057,8 @@ init_thread_db_ops (void)
   thread_db_ops.to_extra_thread_info = thread_db_extra_thread_info;
   thread_db_ops.to_get_ada_task_ptid = thread_db_get_ada_task_ptid;
   thread_db_ops.to_magic = OPS_MAGIC;
+
+  complete_target_initialization (&thread_db_ops);
 }
 
 /* Provide a prototype to silence -Wmissing-prototypes.  */
@@ -2066,7 +2068,6 @@ void
 _initialize_thread_db (void)
 {
   init_thread_db_ops ();
-  add_target (&thread_db_ops);
 
   /* Defer loading of libthread_db.so until inferior is running.
      This allows gdb to load correct libthread_db for a given
diff --git a/gdb/target.c b/gdb/target.c
index 920f916..5bf8059 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -381,16 +381,12 @@ target_has_execution_current (void)
   return target_has_execution_1 (inferior_ptid);
 }
 
-/* Add possible target architecture T to the list and add a new
-   command 'target T->to_shortname'.  Set COMPLETER as the command's
-   completer if not NULL.  */
+/* Complete initialization of T.  This ensures that various fields in
+   T are set, if needed by the target implementation.  */
 
 void
-add_target_with_completer (struct target_ops *t,
-			   completer_ftype *completer)
+complete_target_initialization (struct target_ops *t)
 {
-  struct cmd_list_element *c;
-
   /* Provide default values for all "must have" methods.  */
   if (t->to_xfer_partial == NULL)
     t->to_xfer_partial = default_xfer_partial;
@@ -409,6 +405,19 @@ add_target_with_completer (struct target_ops *t,
 
   if (t->to_has_execution == NULL)
     t->to_has_execution = (int (*) (struct target_ops *, ptid_t)) return_zero;
+}
+
+/* Add possible target architecture T to the list and add a new
+   command 'target T->to_shortname'.  Set COMPLETER as the command's
+   completer if not NULL.  */
+
+void
+add_target_with_completer (struct target_ops *t,
+			   completer_ftype *completer)
+{
+  struct cmd_list_element *c;
+
+  complete_target_initialization (t);
 
   if (!target_structs)
     {
diff --git a/gdb/target.h b/gdb/target.h
index 1bf716e..00dad64 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1837,6 +1837,9 @@ int target_verify_memory (const gdb_byte *data,
 
 /* Routines for maintenance of the target structures...
 
+   complete_target_initialization: Finalize a target_ops by filling in
+   any fields needed by the target implementation.
+
    add_target:   Add a target to the list of all possible targets.
 
    push_target:  Make this target the top of the stack of currently used
@@ -1855,6 +1858,8 @@ extern void add_target (struct target_ops *);
 extern void add_target_with_completer (struct target_ops *t,
 				       completer_ftype *completer);
 
+extern void complete_target_initialization (struct target_ops *t);
+
 /* Adds a command ALIAS for target T and marks it deprecated.  This is useful
    for maintaining backwards compatibility when renaming targets.  */
 
-- 
1.8.1.4

Re: RFC: don't call add_target for thread_db_ops

[Pedro Alves]

On 07/02/2013 05:37 PM, Tom Tromey wrote:

> 	* linux-thread-db.c (init_thread_db_ops): Call
> 	complete_target_initialization.
> 	(_initialize_thread_db): Don't call add_target.
> 	* target.c (complete_target_initialization): New function.
> 	(add_target_with_completer): Call it.
> 	* target.h (complete_target_initialization): Declare.

This is OK.

Really all the thread_stratum targets should be likewise adjusted:

$ grep "to_stratum.*thread_stratum;" * -l | xargs grep add_target
aix-thread.c:  add_target (&aix_thread_ops);
bsd-uthread.c:  add_target (bsd_uthread_target ());
dec-thread.c:  add_target (&dec_thread_ops);
linux-thread-db.c:  add_target (&thread_db_ops);
ravenscar-thread.c:  add_target (&ravenscar_ops);
sol-thread.c:  add_target (&sol_thread_ops);

-- 
Pedro Alves

Re: RFC: don't call add_target for thread_db_ops

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

>> * linux-thread-db.c (init_thread_db_ops): Call
>> complete_target_initialization.
>> (_initialize_thread_db): Don't call add_target.
>> * target.c (complete_target_initialization): New function.
>> (add_target_with_completer): Call it.
>> * target.h (complete_target_initialization): Declare.

Pedro> This is OK.

Thanks.  I'm checking it in now.

Tom

Re: RFC: don't call add_target for thread_db_ops

[Pedro Alves]

On 07/25/2013 03:14 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
>>> * linux-thread-db.c (init_thread_db_ops): Call
>>> complete_target_initialization.
>>> (_initialize_thread_db): Don't call add_target.
>>> * target.c (complete_target_initialization): New function.
>>> (add_target_with_completer): Call it.
>>> * target.h (complete_target_initialization): Declare.
> 
> Pedro> This is OK.
> 
> Thanks.  I'm checking it in now.

Thanks.  Do you plan on doing the same to the remaining
thread_stratum targets?  If not, I'll do it at some point.
I'd prefer not leaving the incomplete transition in
place; it's bound to confuse someone later on.  I wouldn't
worry much if an affected target goes untested.

-- 
Pedro Alves

Re: RFC: don't call add_target for thread_db_ops

[Tom Tromey]

Pedro> Thanks.  Do you plan on doing the same to the remaining
Pedro> thread_stratum targets?  If not, I'll do it at some point.
Pedro> I'd prefer not leaving the incomplete transition in
Pedro> place; it's bound to confuse someone later on.  I wouldn't
Pedro> worry much if an affected target goes untested.

I wasn't planning to, but my main concern was the testing; I will write
the patch today.

Tom

Re: RFC: don't call add_target for thread_db_ops

[Pedro Alves]

On 07/25/2013 05:31 PM, Tom Tromey wrote:
> Pedro> Thanks.  Do you plan on doing the same to the remaining
> Pedro> thread_stratum targets?  If not, I'll do it at some point.
> Pedro> I'd prefer not leaving the incomplete transition in
> Pedro> place; it's bound to confuse someone later on.  I wouldn't
> Pedro> worry much if an affected target goes untested.
> 
> I wasn't planning to, but my main concern was the testing; I will write
> the patch today.

Thanks!

-- 
Pedro Alves

Re: RFC: don't call add_target for thread_db_ops

[Tom Tromey]

Pedro> On 07/25/2013 05:31 PM, Tom Tromey wrote:
Pedro> Thanks.  Do you plan on doing the same to the remaining
Pedro> thread_stratum targets?  If not, I'll do it at some point.
Pedro> I'd prefer not leaving the incomplete transition in
Pedro> place; it's bound to confuse someone later on.  I wouldn't
Pedro> worry much if an affected target goes untested.

Tom> I wasn't planning to, but my main concern was the testing; I will write
Tom> the patch today.

Pedro> Thanks!

Here you go.

Tom

This removes a few more erroneous calls to add_target.  These calls
end up installing the target in a user-visible way; but these targets
are all auto-activated and, I think, should never be explicitly
requested.

I have no way to test these.

CCing Ulrich for the SPU case.

	* aix-thread.c (_initialize_aix_thread): Use
	complete_target_initialization.
	* bsd-uthread.c (_initialize_bsd_uthread): Use
	complete_target_initialization.
	* dec-thread.c (_initialize_dec_thread): Use
	complete_target_initialization.
	* ravenscar-thread.c (_initialize_ravenscar): Use
	complete_target_initialization.
	* sol-thread.c (_initialize_sol_thread): Use
	complete_target_initialization.
	* spu-multiarch.c (_initialize_spu_multiarch): Use
	complete_target_initialization.
---
 gdb/aix-thread.c       | 2 +-
 gdb/bsd-uthread.c      | 2 +-
 gdb/dec-thread.c       | 2 +-
 gdb/ravenscar-thread.c | 2 +-
 gdb/sol-thread.c       | 2 +-
 gdb/spu-multiarch.c    | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index fd1d69b..70f9a3e 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -1837,7 +1837,7 @@ void
 _initialize_aix_thread (void)
 {
   init_aix_thread_ops ();
-  add_target (&aix_thread_ops);
+  complete_target_initialization (&aix_thread_ops);
 
   /* Notice when object files get loaded and unloaded.  */
   observer_attach_new_objfile (new_objfile);
diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
index 6a07985..0a2ea81 100644
--- a/gdb/bsd-uthread.c
+++ b/gdb/bsd-uthread.c
@@ -547,7 +547,7 @@ extern initialize_file_ftype _initialize_bsd_uthread;
 void
 _initialize_bsd_uthread (void)
 {
-  add_target (bsd_uthread_target ());
+  complete_target_initialization (bsd_uthread_target ());
 
   bsd_uthread_data = gdbarch_data_register_pre_init (bsd_uthread_init);
 
diff --git a/gdb/dec-thread.c b/gdb/dec-thread.c
index 27cf004..f07721d 100644
--- a/gdb/dec-thread.c
+++ b/gdb/dec-thread.c
@@ -727,7 +727,7 @@ void
 _initialize_dec_thread (void)
 {
   init_dec_thread_ops ();
-  add_target (&dec_thread_ops);
+  complete_target_initialization (&dec_thread_ops);
 
   observer_attach_new_objfile (dec_thread_new_objfile_observer);
 
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 0a3100d..26b8171 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -433,7 +433,7 @@ _initialize_ravenscar (void)
      ravenscar ops if needed.  */
   observer_attach_inferior_created (ravenscar_inferior_created);
 
-  add_target (&ravenscar_ops);
+  complete_target_initialization (&ravenscar_ops);
 
   add_prefix_cmd ("ravenscar", no_class, set_ravenscar_command,
                   _("Prefix command for changing Ravenscar-specific settings"),
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index f1b29a0..c9abaf7 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -1276,7 +1276,7 @@ _initialize_sol_thread (void)
   resolve (td_thr_getgregs);
   resolve (td_thr_setgregs);
 
-  add_target (&sol_thread_ops);
+  complete_target_initialization (&sol_thread_ops);
 
   add_cmd ("sol-threads", class_maintenance, info_solthreads,
 	   _("Show info on Solaris user threads."), &maintenanceinfolist);
diff --git a/gdb/spu-multiarch.c b/gdb/spu-multiarch.c
index a74bd30..e93f142 100644
--- a/gdb/spu-multiarch.c
+++ b/gdb/spu-multiarch.c
@@ -410,7 +410,7 @@ _initialize_spu_multiarch (void)
 {
   /* Install ourselves on the target stack.  */
   init_spu_ops ();
-  add_target (&spu_ops);
+  complete_target_initialization (&spu_ops);
 
   /* Install observers to watch for SPU objects.  */
   observer_attach_inferior_created (spu_multiarch_inferior_created);
-- 
1.8.1.4

Re: RFC: don't call add_target for thread_db_ops

[Ulrich Weigand]

Tom Tromey wrote:

> This removes a few more erroneous calls to add_target.  These calls
> end up installing the target in a user-visible way; but these targets
> are all auto-activated and, I think, should never be explicitly
> requested.
> 
> I have no way to test these.
> 
> CCing Ulrich for the SPU case.

Makes sense to me.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: RFC: don't call add_target for thread_db_ops

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

Tom> This removes a few more erroneous calls to add_target.  These calls
Tom> end up installing the target in a user-visible way; but these targets
Tom> are all auto-activated and, I think, should never be explicitly
Tom> requested.

Tom> I have no way to test these.

I'm checking this in.
If it causes some problem, please let me know and I will try to fix it up.
I don't anticipate anything though.

Tom