Re: [PATCH 00/40] C++ify target_ops, toward multi-target

[Pedro Alves <palves@redhat.com>]

On 04/29/2018 04:22 PM, Simon Marchi wrote:
> On 2018-04-14 03:09 PM, Pedro Alves wrote:
>> Here's another chunk from the multi-target branch.
> 
> Hi Pedro,
> 
> It's kind of difficult to properly review this series, because the important
> non-mechanical bits are lost in a sea of mechanical changes.  

Yeah, sorry about that.

> But I didn't
> find anything fundamental I would change, and all you wrote in commit logs
> made sense to me.
> 
> Maybe one little comment about 40/40 (because I just finished looking at that
> patch): it might be good to add an assertion (the_native_target == nullptr)
> in set_native_target and one in add_target
> (target_factories.find (&t) == target_factories.end ()).  I think it could
> help catch problems if somebody is trying to add a new target or change
> existing ones.

Good idea.  I'm adding this to patch #40:

 gdb/target.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/gdb/target.c b/gdb/target.c
index b957769a3f..824855e7df 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -352,7 +352,11 @@ add_target (const target_info &t, target_open_ftype *func,
 {
   struct cmd_list_element *c;
 
-  target_factories[&t] = func;
+  auto &func_slot = target_factories[&t];
+  if (func_slot != nullptr)
+    internal_error (__FILE__, __LINE__,
+		    "target already added (\"%s\").", t.shortname);
+  func_slot = func;
 
   if (targetlist == NULL)
     add_prefix_cmd ("target", class_run, target_command, _("\
@@ -2447,6 +2451,11 @@ static target_ops *the_native_target;
 void
 set_native_target (target_ops *target)
 {
+  if (the_native_target != NULL)
+    internal_error (__FILE__, __LINE__,
+		    _("native target already set (\"%s\")."),
+		    the_native_target->longname ());
+
   the_native_target = target;
 }

> Otherwise, I would suggest not waiting too long before merging it to
> avoid having to resolve too many conflicts.  Thanks a lot for doing this!
Alright, I wrote the missing ChangeLogs today, and will proceed with
squashing the patches that need to be squashed and getting it all in.

Thanks,
Pedro Alves