[PATCH] fix to_open debug setting

[PATCH] fix to_open debug setting

[Tom Tromey]

This is a follow-on to the patch to auto-generate target debug methods.

While working on that patch I noticed that the to_open debug setting
will never work.  There is no path by which debug_to_open can be
called.

This patch fixes the problem by using a generic function as the
implementation of the various "target" subcommands, and then putting
the debug printing there.

This is also a tiny step toward fixing PR 7250 (and apparently why
command contexts were introduced).

Built and regtested on x86-64 Fedora 20.

2014-07-21  Tom Tromey  <tromey@redhat.com>

	* target.c (open_target): New function.
	(add_target_with_completer): Use set_cmd_sfunc, set_cmd_context.
	(debug_to_open): Remove.
	(setup_target_debug): Update.
---
 gdb/ChangeLog |  7 +++++++
 gdb/target.c  | 28 ++++++++++++++++------------
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/gdb/target.c b/gdb/target.c
index b9310cf..0db21e2 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -118,8 +118,6 @@ static struct target_ops debug_target;
 
 static void init_dummy_target (void);
 
-static void debug_to_open (char *, int);
-
 /* Pointer to array of target architecture structures; the size of the
    array; the current index into the array; the allocated size of the
    array.  */
@@ -345,6 +343,19 @@ complete_target_initialization (struct target_ops *t)
   install_delegators (t);
 }
 
+/* This is used to implement the various target commands.  */
+
+static void
+open_target (char *args, int from_tty, struct cmd_list_element *command)
+{
+  struct target_ops *ops = get_cmd_context (command);
+
+  if (targetdebug)
+    fprintf_unfiltered (gdb_stdlog, "target_open (%s, %d)\n", args, from_tty);
+
+  ops->to_open (args, from_tty);
+}
+
 /* 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.  */
@@ -380,8 +391,9 @@ Remaining arguments are interpreted by the target protocol.  For more\n\
 information on the arguments for a particular protocol, type\n\
 `help target ' followed by the protocol name."),
 		    &targetlist, "target ", 0, &cmdlist);
-  c = add_cmd (t->to_shortname, no_class, t->to_open, t->to_doc,
-	       &targetlist);
+  c = add_cmd (t->to_shortname, no_class, NULL, t->to_doc, &targetlist);
+  set_cmd_sfunc (c, open_target);
+  set_cmd_context (c, t);
   if (completer != NULL)
     set_cmd_completer (c, completer);
 }
@@ -2946,13 +2958,6 @@ init_dummy_target (void)
   install_dummy_methods (&dummy_target);
 }
 \f
-static void
-debug_to_open (char *args, int from_tty)
-{
-  debug_target.to_open (args, from_tty);
-
-  fprintf_unfiltered (gdb_stdlog, "target_open (%s, %d)\n", args, from_tty);
-}
 
 void
 target_close (struct target_ops *targ)
@@ -3397,7 +3402,6 @@ setup_target_debug (void)
 {
   memcpy (&debug_target, &current_target, sizeof debug_target);
 
-  current_target.to_open = debug_to_open;
   init_debug_target (&current_target);
 }
 \f
-- 
1.9.3

Re: [PATCH] fix to_open debug setting

[Tom Tromey]

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

Tom> This patch fixes the problem by using a generic function as the
Tom> implementation of the various "target" subcommands, and then putting
Tom> the debug printing there.

Later I happened to notice that add_deprecated_target_alias could use
the same treatment.  I'll send the new patch tomorrow.

Tom

Re: [PATCH] fix to_open debug setting

[Tom Tromey]

Tom> Later I happened to notice that add_deprecated_target_alias could use
Tom> the same treatment.  I'll send the new patch tomorrow.

Here it is.

Tom

2014-07-21  Tom Tromey  <tromey@redhat.com>

	* target.c (open_target): New function.
	(add_target_with_completer, add_deprecated_target_alias): Use
	set_cmd_sfunc, set_cmd_context.
	(debug_to_open): Remove.
	(setup_target_debug): Update.

diff --git a/gdb/target.c b/gdb/target.c
index b9310cf..42d7800 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -118,8 +118,6 @@ static struct target_ops debug_target;
 
 static void init_dummy_target (void);
 
-static void debug_to_open (char *, int);
-
 /* Pointer to array of target architecture structures; the size of the
    array; the current index into the array; the allocated size of the
    array.  */
@@ -345,6 +343,19 @@ complete_target_initialization (struct target_ops *t)
   install_delegators (t);
 }
 
+/* This is used to implement the various target commands.  */
+
+static void
+open_target (char *args, int from_tty, struct cmd_list_element *command)
+{
+  struct target_ops *ops = get_cmd_context (command);
+
+  if (targetdebug)
+    fprintf_unfiltered (gdb_stdlog, "target_open (%s, %d)\n", args, from_tty);
+
+  ops->to_open (args, from_tty);
+}
+
 /* 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.  */
@@ -380,8 +391,9 @@ Remaining arguments are interpreted by the target protocol.  For more\n\
 information on the arguments for a particular protocol, type\n\
 `help target ' followed by the protocol name."),
 		    &targetlist, "target ", 0, &cmdlist);
-  c = add_cmd (t->to_shortname, no_class, t->to_open, t->to_doc,
-	       &targetlist);
+  c = add_cmd (t->to_shortname, no_class, NULL, t->to_doc, &targetlist);
+  set_cmd_sfunc (c, open_target);
+  set_cmd_context (c, t);
   if (completer != NULL)
     set_cmd_completer (c, completer);
 }
@@ -404,7 +416,9 @@ add_deprecated_target_alias (struct target_ops *t, char *alias)
 
   /* If we use add_alias_cmd, here, we do not get the deprecated warning,
      see PR cli/15104.  */
-  c = add_cmd (alias, no_class, t->to_open, t->to_doc, &targetlist);
+  c = add_cmd (alias, no_class, NULL, t->to_doc, &targetlist);
+  set_cmd_sfunc (c, open_target);
+  set_cmd_context (c, t);
   alt = xstrprintf ("target %s", t->to_shortname);
   deprecate_cmd (c, alt);
 }
@@ -2946,13 +2960,6 @@ init_dummy_target (void)
   install_dummy_methods (&dummy_target);
 }
 \f
-static void
-debug_to_open (char *args, int from_tty)
-{
-  debug_target.to_open (args, from_tty);
-
-  fprintf_unfiltered (gdb_stdlog, "target_open (%s, %d)\n", args, from_tty);
-}
 
 void
 target_close (struct target_ops *targ)
@@ -3397,7 +3404,6 @@ setup_target_debug (void)
 {
   memcpy (&debug_target, &current_target, sizeof debug_target);
 
-  current_target.to_open = debug_to_open;
   init_debug_target (&current_target);
 }
 \f

Re: [PATCH] fix to_open debug setting

[Pedro Alves]

On 07/22/2014 04:27 PM, Tom Tromey wrote:
> Tom> Later I happened to notice that add_deprecated_target_alias could use
> Tom> the same treatment.  I'll send the new patch tomorrow.
> 
> Here it is.

Looks good to me.

> +/* This is used to implement the various target commands.  */
> +
> +static void
> +open_target (char *args, int from_tty, struct cmd_list_element *command)
> +{
> +  struct target_ops *ops = get_cmd_context (command);
> +
> +  if (targetdebug)
> +    fprintf_unfiltered (gdb_stdlog, "target_open (%s, %d)\n", args, from_tty);

I think it'd be nice if this included the target's name, like your
generated debug methods.

Thanks,
-- 
Pedro Alves

Re: [PATCH] fix to_open debug setting

[Tom Tromey]

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

Pedro> I think it'd be nice if this included the target's name, like your
Pedro> generated debug methods.

Here's a new version.

I think it would also be nice to indent target debug output according to
the depth of the calls.  Right now I see:

    (gdb) target remote :9999
    -> remote->to_open (...)
    Remote debugging using :9999
    -> remote->to_can_async_p (...)
    <- remote->to_can_async_p (0x1dcf6c0) = 1
    -> remote->to_can_async_p (...)
    <- remote->to_can_async_p (0x1dcf6c0) = 1
    -> remote->to_program_signals (...)
    -> remote->to_can_async_p (...)
    [... lots more ...]
    <- remote->to_open (:9999, 1)
    (gdb)

Now that this is mostly automated we could more easily indent those
lines to get a better sense of the control flow.

However I think that's a patch for another day.

Tom

b/gdb/ChangeLog:
2014-07-24  Tom Tromey  <tromey@redhat.com>

	* target.c (open_target): New function.
	(add_target_with_completer, add_deprecated_target_alias): Use
	set_cmd_sfunc, set_cmd_context.
	(debug_to_open): Remove.
	(setup_target_debug): Update.

diff --git a/gdb/target.c b/gdb/target.c
index d9b471b..4358722 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -118,8 +118,6 @@ static struct target_ops debug_target;
 
 static void init_dummy_target (void);
 
-static void debug_to_open (char *, int);
-
 /* Pointer to array of target architecture structures; the size of the
    array; the current index into the array; the allocated size of the
    array.  */
@@ -345,6 +343,24 @@ complete_target_initialization (struct target_ops *t)
   install_delegators (t);
 }
 
+/* This is used to implement the various target commands.  */
+
+static void
+open_target (char *args, int from_tty, struct cmd_list_element *command)
+{
+  struct target_ops *ops = get_cmd_context (command);
+
+  if (targetdebug)
+    fprintf_unfiltered (gdb_stdlog, "-> %s->to_open (...)\n",
+			ops->to_shortname);
+
+  ops->to_open (args, from_tty);
+
+  if (targetdebug)
+    fprintf_unfiltered (gdb_stdlog, "<- %s->to_open (%s, %d)\n",
+			ops->to_shortname, args, from_tty);
+}
+
 /* 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.  */
@@ -380,8 +396,9 @@ Remaining arguments are interpreted by the target protocol.  For more\n\
 information on the arguments for a particular protocol, type\n\
 `help target ' followed by the protocol name."),
 		    &targetlist, "target ", 0, &cmdlist);
-  c = add_cmd (t->to_shortname, no_class, t->to_open, t->to_doc,
-	       &targetlist);
+  c = add_cmd (t->to_shortname, no_class, NULL, t->to_doc, &targetlist);
+  set_cmd_sfunc (c, open_target);
+  set_cmd_context (c, t);
   if (completer != NULL)
     set_cmd_completer (c, completer);
 }
@@ -404,7 +421,9 @@ add_deprecated_target_alias (struct target_ops *t, char *alias)
 
   /* If we use add_alias_cmd, here, we do not get the deprecated warning,
      see PR cli/15104.  */
-  c = add_cmd (alias, no_class, t->to_open, t->to_doc, &targetlist);
+  c = add_cmd (alias, no_class, NULL, t->to_doc, &targetlist);
+  set_cmd_sfunc (c, open_target);
+  set_cmd_context (c, t);
   alt = xstrprintf ("target %s", t->to_shortname);
   deprecate_cmd (c, alt);
 }
@@ -2963,13 +2982,6 @@ init_dummy_target (void)
   install_dummy_methods (&dummy_target);
 }
 \f
-static void
-debug_to_open (char *args, int from_tty)
-{
-  debug_target.to_open (args, from_tty);
-
-  fprintf_unfiltered (gdb_stdlog, "target_open (%s, %d)\n", args, from_tty);
-}
 
 void
 target_close (struct target_ops *targ)
@@ -3415,7 +3427,6 @@ setup_target_debug (void)
 {
   memcpy (&debug_target, &current_target, sizeof debug_target);
 
-  current_target.to_open = debug_to_open;
   init_debug_target (&current_target);
 }
 \f

Re: [PATCH] fix to_open debug setting

[Pedro Alves]

On 07/24/2014 04:15 PM, Tom Tromey wrote:

> b/gdb/ChangeLog:
> 2014-07-24  Tom Tromey  <tromey@redhat.com>
> 
> 	* target.c (open_target): New function.
> 	(add_target_with_completer, add_deprecated_target_alias): Use
> 	set_cmd_sfunc, set_cmd_context.
> 	(debug_to_open): Remove.
> 	(setup_target_debug): Update.
> 

Looks great.  Thanks!

-- 
Thanks,
Pedro Alves

Re: [PATCH] fix to_open debug setting

[Tom Tromey]

Pedro> I think it'd be nice if this included the target's name, like your
Pedro> generated debug methods.

Tom> Here's a new version.

I'm checking this in now.

Tom