Re: [PATCH] interpreter-exec error path

[Nick Roberts <nickrob at snap dot net dot nz>]

> If any of the commands executed through "interpreter-exec" fails, the
> "quiet" value is not restored for the interpreter.  This may result in
> something like the following.


> (gdb) interpreter-exec console rubbish
> Undefined command: "rubbish".  Try "help".
> error in command: "rubbish".
> ...

It's a somewhat circular example but I get your point.

> The attached patch will handle the error more correctly.

It's more convenient to post such a small patch just as text

> 2006-09-09  Andrzej Zaborowski  <balrog@zabor.org>
> 	* interps.c (interpreter_exec_cmd): Restore interpreter properties.

> --- gdb-orig/gdb/interps.c	2006-09-08 06:10:15.000000000 +0000
> +++ gdb/gdb/interps.c	2006-09-08 06:04:20.000000000 +0000
> @@ -402,9 +402,9 @@ interpreter_exec_cmd (char *args, int fr
>        if (e.reason < 0)
>  	{
>  	  interp_set (old_interp);
> -	  interp_set_quiet (interp_to_use, old_quiet);
> +	  interp_set_quiet (interp_to_use, use_quiet);
> +	  interp_set_quiet (old_interp, old_quiet);
>  	  error (_("error in command: \"%s\"."), prules[i]);
> -	  break;
>  	}
>      }

Yes, I think this does what Andrew Cagney intended but the underlying
interpreter has already signalled the exception so I think it could be
handled normally:

*** interps.c	13 Jul 2006 21:03:38 +1200	1.17
--- interps.c	12 Sep 2006 09:25:17 +1200	
***************
*** 399,411 ****
    for (i = 1; i < nrules; i++)
      {
        struct gdb_exception e = interp_exec (interp_to_use, prules[i]);
!       if (e.reason < 0)
! 	{
! 	  interp_set (old_interp);
! 	  interp_set_quiet (interp_to_use, old_quiet);
! 	  error (_("error in command: \"%s\"."), prules[i]);
! 	  break;
! 	}
      }
  
    interp_set (old_interp);
--- 399,405 ----
    for (i = 1; i < nrules; i++)
      {
        struct gdb_exception e = interp_exec (interp_to_use, prules[i]);
!       if (e.reason < 0) break;
      }
  
    interp_set (old_interp);

Taking things a step further, I see that mi_interpreter_exec always returns
exception_none so cli_interpreter_exec could do the same (patch below).  The
command interpreter-exec can handle a list of commands, this would mean if the
first fails, GDB will still handle the subsequent commands.  This is currently
true for mi e.g
 
(gdb) i interpreter-exec mi -ttd -environment-pwd
^error,msg="Undefined MI command: ttd"
(gdb)
^done,cwd="/home/nickrob"
(gdb)
(gdb)

-- 
Nick                                           http://www.inet.net.nz/~nickrob


*** interps.c	13 Jul 2006 21:03:38 +1200	1.17
--- interps.c	12 Sep 2006 10:02:34 +1200	
***************
*** 397,412 ****
      error (_("Could not switch to interpreter \"%s\"."), prules[0]);
  
    for (i = 1; i < nrules; i++)
!     {
!       struct gdb_exception e = interp_exec (interp_to_use, prules[i]);
!       if (e.reason < 0)
! 	{
! 	  interp_set (old_interp);
! 	  interp_set_quiet (interp_to_use, old_quiet);
! 	  error (_("error in command: \"%s\"."), prules[i]);
! 	  break;
! 	}
!     }
  
    interp_set (old_interp);
    interp_set_quiet (interp_to_use, use_quiet);
--- 397,403 ----
      error (_("Could not switch to interpreter \"%s\"."), prules[0]);
  
    for (i = 1; i < nrules; i++)
!       interp_exec (interp_to_use, prules[i]);
  
    interp_set (old_interp);
    interp_set_quiet (interp_to_use, use_quiet);


*** cli-interp.c	18 Dec 2005 11:40:17 +1300	1.11
--- cli-interp.c	12 Sep 2006 10:03:35 +1200	
***************
*** 96,102 ****
  cli_interpreter_exec (void *data, const char *command_str)
  {
    struct ui_file *old_stream;
-   struct gdb_exception result;
  
    /* FIXME: cagney/2003-02-01: Need to const char *propogate
       safe_execute_command.  */
--- 96,101 ----
***************
*** 109,117 ****
       It is important that it gets reset everytime, since the user could
       set gdb to use a different interpreter.  */
    old_stream = cli_out_set_stream (cli_uiout, gdb_stdout);
!   result = safe_execute_command (cli_uiout, str, 1);
    cli_out_set_stream (cli_uiout, old_stream);
!   return result;
  }
  
  static void
--- 108,116 ----
       It is important that it gets reset everytime, since the user could
       set gdb to use a different interpreter.  */
    old_stream = cli_out_set_stream (cli_uiout, gdb_stdout);
!   safe_execute_command (cli_uiout, str, 1);
    cli_out_set_stream (cli_uiout, old_stream);
!   return exception_none;
  }
  
  static void