[PATCH] Give every interpreter a command_loop_proc.

[PATCH] Give every interpreter a command_loop_proc.

[Andrew Burgess]

This patch simplifies gdb/interps.c:current_interp_command_loop
by giving every interpreter a command_loop_proc.

Of the 4 interpreters I'm aware of mi and gdbtk already install
a command loop proc, while cli and tui don't instead using
fall-through code in current_interp_command_loop.

I've given cli and tui a command loop proc, and removed the
fall-through code.

This change does rely on "current_interpreter" never being NULL,
I believe this is true, and no tests failed here (x86-64 linux),
but if anyone know different please let me know.

OK to apply?

Thanks,
Andrew


2013-09-05  Andrew Burgess  <aburgess@broadcom.com>

	* gdb/cli/cli-interp.c (_initialize_cli_interp): Add a
	command_loop_proc to interp_procs.
	* gdb/event-top.c (cli_command_loop): Change signature to match
	interp_command_loop_ftype.
	* gdb/event-top.h (cli_command_loop): Same.
	* gdb/interps.c (interp_new): Require every interpreter to have a
	command_loop_proc.
	(current_interp_command_loop): Just call the command_loop_proc on
	the current interpreter.
	* gdb/tui/tui-interp.c (_initialize_tui_interp): Add a
	command_loop_proc to interp_procs.

diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c
index 1003cc7..ac3d9a8 100644
--- a/gdb/cli/cli-interp.c
+++ b/gdb/cli/cli-interp.c
@@ -151,7 +151,9 @@ _initialize_cli_interp (void)
     cli_interpreter_suspend,	/* suspend_proc */
     cli_interpreter_exec,	/* exec_proc */
     cli_interpreter_display_prompt_p,	/* prompt_proc_p */
-    cli_ui_out			/* ui_out_proc */
+    cli_ui_out,			/* ui_out_proc */
+    NULL,                       /* set_logging_proc */
+    cli_command_loop            /* command_loop_proc */
   };
   struct interp *cli_interp;
 
diff --git a/gdb/event-top.c b/gdb/event-top.c
index f1d55b3..1f2e6fb 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -167,9 +167,11 @@ rl_callback_read_char_wrapper (gdb_client_data client_data)
 }
 
 /* Initialize all the necessary variables, start the event loop,
-   register readline, and stdin, start the loop.  */
+   register readline, and stdin, start the loop.  The DATA is the
+   interpreter data cookie, ignored for now.  */
+
 void
-cli_command_loop (void)
+cli_command_loop (void *data)
 {
   display_gdb_prompt (0);
 
diff --git a/gdb/event-top.h b/gdb/event-top.h
index e6166f6..97142d4 100644
--- a/gdb/event-top.h
+++ b/gdb/event-top.h
@@ -62,6 +62,6 @@ extern void (*input_handler) (char *);
 extern int input_fd;
 extern void (*after_char_processing_hook) (void);
 
-extern void cli_command_loop (void);
+extern void cli_command_loop (void *);
 
 #endif
diff --git a/gdb/interps.c b/gdb/interps.c
index 33e0f72..7f19385 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -101,6 +101,9 @@ interp_new (const char *name, const struct interp_procs *procs)
   new_interp->procs = procs;
   new_interp->inited = 0;
 
+  /* Check for required procs.  */
+  gdb_assert (procs->command_loop_proc != NULL);
+
   return new_interp;
 }
 
@@ -319,13 +322,9 @@ current_interp_display_prompt_p (void)
 void
 current_interp_command_loop (void)
 {
-  /* Somewhat messy.  For the moment prop up all the old ways of
-     selecting the command loop.  */
-  if (current_interpreter != NULL
-      && current_interpreter->procs->command_loop_proc != NULL)
-    current_interpreter->procs->command_loop_proc (current_interpreter->data);
-  else
-    cli_command_loop ();
+  gdb_assert (current_interpreter != NULL);
+
+  current_interpreter->procs->command_loop_proc (current_interpreter->data);
 }
 
 int
diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c
index 42526e6..1095220 100644
--- a/gdb/tui/tui-interp.c
+++ b/gdb/tui/tui-interp.c
@@ -153,6 +153,8 @@ _initialize_tui_interp (void)
     tui_exec,
     tui_display_prompt_p,
     tui_ui_out,
+    NULL,
+    cli_command_loop
   };
   struct interp *tui_interp;
 

Re: [PATCH] Give every interpreter a command_loop_proc.

[Pedro Alves]

On 09/05/2013 05:59 PM, Andrew Burgess wrote:

> 2013-09-05  Andrew Burgess  <aburgess@broadcom.com>
> 
> 	* gdb/cli/cli-interp.c (_initialize_cli_interp): Add a
> 	command_loop_proc to interp_procs.
> 	* gdb/event-top.c (cli_command_loop): Change signature to match
> 	interp_command_loop_ftype.
> 	* gdb/event-top.h (cli_command_loop): Same.
> 	* gdb/interps.c (interp_new): Require every interpreter to have a
> 	command_loop_proc.
> 	(current_interp_command_loop): Just call the command_loop_proc on
> 	the current interpreter.
> 	* gdb/tui/tui-interp.c (_initialize_tui_interp): Add a
> 	command_loop_proc to interp_procs.

s,gdb/,, in all the above.

OK.

Thanks!

-- 
Pedro Alves

Re: [PATCH] Give every interpreter a command_loop_proc.

[Andrew Burgess]

On 05/09/2013 6:13 PM, Pedro Alves wrote:
> On 09/05/2013 05:59 PM, Andrew Burgess wrote:
> 
>> 2013-09-05  Andrew Burgess  <aburgess@broadcom.com>
>>
>> 	* gdb/cli/cli-interp.c (_initialize_cli_interp): Add a
>> 	command_loop_proc to interp_procs.
>> 	* gdb/event-top.c (cli_command_loop): Change signature to match
>> 	interp_command_loop_ftype.
>> 	* gdb/event-top.h (cli_command_loop): Same.
>> 	* gdb/interps.c (interp_new): Require every interpreter to have a
>> 	command_loop_proc.
>> 	(current_interp_command_loop): Just call the command_loop_proc on
>> 	the current interpreter.
>> 	* gdb/tui/tui-interp.c (_initialize_tui_interp): Add a
>> 	command_loop_proc to interp_procs.
> 
> s,gdb/,, in all the above.
> 
> OK.

Committed with the above fix.

Thanks Pedro & Stan for the review.

Andrew

Re: [PATCH] Give every interpreter a command_loop_proc.

[Stan Shebs]

On 9/5/13 9:59 AM, Andrew Burgess wrote:
> This patch simplifies gdb/interps.c:current_interp_command_loop
> by giving every interpreter a command_loop_proc.
> 
> Of the 4 interpreters I'm aware of mi and gdbtk already install
> a command loop proc, while cli and tui don't instead using
> fall-through code in current_interp_command_loop.
> 
> I've given cli and tui a command loop proc, and removed the
> fall-through code.
> 
> This change does rely on "current_interpreter" never being NULL,
> I believe this is true, and no tests failed here (x86-64 linux),
> but if anyone know different please let me know.
> 
> OK to apply?

I like it!  This is OK with one caveat:

> 	* gdb/cli/cli-interp.c (_initialize_cli_interp): Add a
> 	command_loop_proc to interp_procs.

No redundant "gdb/", please.

Stan
stan@codesourcery.com