[17/21] gdb: pass current_ui down to interp_set

Message ID 20230908190227.96319-18-simon.marchi@efficios.com
State New
Headers
Series ui / interp cleansup |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed

Commit Message

Simon Marchi Sept. 8, 2023, 6:23 p.m. UTC
  In preparation for making interp_set a method of struct ui.  No behavior
changes expected.

Change-Id: I507c7701438967502d714ecda99401d785806cab
---
 gdb/interps.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)
  

Comments

Andrew Burgess Sept. 12, 2023, 10:54 a.m. UTC | #1
Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

> In preparation for making interp_set a method of struct ui.  No behavior
> changes expected.
>
> Change-Id: I507c7701438967502d714ecda99401d785806cab
> ---
>  gdb/interps.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/gdb/interps.c b/gdb/interps.c
> index 0575128b8124..18758f1f7af6 100644
> --- a/gdb/interps.c
> +++ b/gdb/interps.c
> @@ -92,24 +92,24 @@ find_interp_factory (const char *name)
>     are caused by CLI commands.  */
>  
>  static void
> -interp_set (struct interp *interp, bool top_level)
> +interp_set (ui *ui, struct interp *interp, bool top_level)
>  {

Given where you're going, I'm fine with this.  If we planned to stop
here then I'd say, lets use interp->m_ui instead of passing in a
separate ui parameter, but I see why this helps.

Thanks,
Andrew

> -  struct interp *old_interp = current_ui->current_interpreter;
> +  struct interp *old_interp = ui->current_interpreter;
>  
>    /* If we already have an interpreter, then trying to
>       set top level interpreter is kinda pointless.  */
> -  gdb_assert (!top_level || !current_ui->current_interpreter);
> -  gdb_assert (!top_level || !current_ui->top_level_interpreter);
> +  gdb_assert (!top_level || !ui->current_interpreter);
> +  gdb_assert (!top_level || !ui->top_level_interpreter);
>  
>    if (old_interp != NULL)
>      {
> -      current_ui->m_current_uiout->flush ();
> +      ui->m_current_uiout->flush ();
>        old_interp->suspend ();
>      }
>  
> -  current_ui->current_interpreter = interp;
> +  ui->current_interpreter = interp;
>    if (top_level)
> -    current_ui->top_level_interpreter = interp;
> +    ui->top_level_interpreter = interp;
>  
>    if (interpreter_p != interp->name ())
>      interpreter_p = interp->name ();
> @@ -127,7 +127,7 @@ interp_set (struct interp *interp, bool top_level)
>      }
>  
>    /* Do this only after the interpreter is initialized.  */
> -  current_ui->m_current_uiout = interp->interp_ui_out ();
> +  ui->m_current_uiout = interp->interp_ui_out ();
>  
>    /* Clear out any installed interpreter hooks/event handlers.  */
>    clear_interpreter_hooks ();
> @@ -151,7 +151,7 @@ set_top_level_interpreter (const char *name)
>    if (interp == NULL)
>      error (_("Interpreter `%s' unrecognized"), name);
>    /* Install it.  */
> -  interp_set (interp, true);
> +  interp_set (current_ui, interp, true);
>  }
>  
>  void
> @@ -267,10 +267,10 @@ interpreter_exec_cmd (const char *args, int from_tty)
>    if (interp_to_use == NULL)
>      error (_("Could not find interpreter \"%s\"."), prules[0]);
>  
> -  interp_set (interp_to_use, false);
> +  interp_set (current_ui, interp_to_use, false);
>    SCOPE_EXIT
>      {
> -      interp_set (old_interp, false);
> +      interp_set (current_ui, old_interp, false);
>      };
>  
>    for (i = 1; i < nrules; i++)
> -- 
> 2.42.0
  
Simon Marchi Sept. 12, 2023, 5:17 p.m. UTC | #2
On 9/12/23 06:54, Andrew Burgess wrote:
> Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
>> In preparation for making interp_set a method of struct ui.  No behavior
>> changes expected.
>>
>> Change-Id: I507c7701438967502d714ecda99401d785806cab
>> ---
>>  gdb/interps.c | 22 +++++++++++-----------
>>  1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/gdb/interps.c b/gdb/interps.c
>> index 0575128b8124..18758f1f7af6 100644
>> --- a/gdb/interps.c
>> +++ b/gdb/interps.c
>> @@ -92,24 +92,24 @@ find_interp_factory (const char *name)
>>     are caused by CLI commands.  */
>>  
>>  static void
>> -interp_set (struct interp *interp, bool top_level)
>> +interp_set (ui *ui, struct interp *interp, bool top_level)
>>  {
> 
> Given where you're going, I'm fine with this.  If we planned to stop
> here then I'd say, lets use interp->m_ui instead of passing in a
> separate ui parameter, but I see why this helps.

Ack.  This patch basically adds the equivalent of "this", in preparation
to making this a method, so it indeed disappears in the next patch.

However, this makes me realize it's possible to do:

  ui->set_current_interpreter (interp)

with an INTERP that doesn't belong to UI, and that would be a mistake
(that's nothing new, it could happen with interp_set today).  Perhaps I
can add an assert in set_current_interpreter to ensure we don't try to
add set an interp on a UI that belongs to another UI.

Simon
  

Patch

diff --git a/gdb/interps.c b/gdb/interps.c
index 0575128b8124..18758f1f7af6 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -92,24 +92,24 @@  find_interp_factory (const char *name)
    are caused by CLI commands.  */
 
 static void
-interp_set (struct interp *interp, bool top_level)
+interp_set (ui *ui, struct interp *interp, bool top_level)
 {
-  struct interp *old_interp = current_ui->current_interpreter;
+  struct interp *old_interp = ui->current_interpreter;
 
   /* If we already have an interpreter, then trying to
      set top level interpreter is kinda pointless.  */
-  gdb_assert (!top_level || !current_ui->current_interpreter);
-  gdb_assert (!top_level || !current_ui->top_level_interpreter);
+  gdb_assert (!top_level || !ui->current_interpreter);
+  gdb_assert (!top_level || !ui->top_level_interpreter);
 
   if (old_interp != NULL)
     {
-      current_ui->m_current_uiout->flush ();
+      ui->m_current_uiout->flush ();
       old_interp->suspend ();
     }
 
-  current_ui->current_interpreter = interp;
+  ui->current_interpreter = interp;
   if (top_level)
-    current_ui->top_level_interpreter = interp;
+    ui->top_level_interpreter = interp;
 
   if (interpreter_p != interp->name ())
     interpreter_p = interp->name ();
@@ -127,7 +127,7 @@  interp_set (struct interp *interp, bool top_level)
     }
 
   /* Do this only after the interpreter is initialized.  */
-  current_ui->m_current_uiout = interp->interp_ui_out ();
+  ui->m_current_uiout = interp->interp_ui_out ();
 
   /* Clear out any installed interpreter hooks/event handlers.  */
   clear_interpreter_hooks ();
@@ -151,7 +151,7 @@  set_top_level_interpreter (const char *name)
   if (interp == NULL)
     error (_("Interpreter `%s' unrecognized"), name);
   /* Install it.  */
-  interp_set (interp, true);
+  interp_set (current_ui, interp, true);
 }
 
 void
@@ -267,10 +267,10 @@  interpreter_exec_cmd (const char *args, int from_tty)
   if (interp_to_use == NULL)
     error (_("Could not find interpreter \"%s\"."), prules[0]);
 
-  interp_set (interp_to_use, false);
+  interp_set (current_ui, interp_to_use, false);
   SCOPE_EXIT
     {
-      interp_set (old_interp, false);
+      interp_set (current_ui, old_interp, false);
     };
 
   for (i = 1; i < nrules; i++)