[RFC/MI] Renumber enum mi_cmd_result

[RFC/MI] Renumber enum mi_cmd_result

[Keith Seitz]

Hi,

I've been looking over Apple's MI changes, so expect to see quite a few
questions/changes/patches/debates/documentation updates/etc in the coming
weeks on MI.

This simple patch renumbers enum mi_cmd_result so that it starts at one
instead of zero. This is necessary because MI_CMD_DONE would be
indistinguishable from an error condition (both would return zero to
catch_errors).

I have run the testsuite before and after this change, and it produces no
differences.

(Okay, this is almost too simple to leave by itself, but, gotta start
somewhere!)

Keith

ChangeLog
2002-05-17  Keith Seitz  <keiths@redhat.com>

        From Jim Ingham <jingham at apple dot com>:
        * mi-cmds.h (enum mi_cmd_result): Start numbering at one instead
        of zero so that errors can be distinguished from other status
        returns.

Patch
Index: mi/mi-cmds.h
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.h,v
retrieving revision 1.5
diff -p -r1.5 mi-cmds.h
*** mi/mi-cmds.h	6 Mar 2001 08:21:45 -0000	1.5
--- mi/mi-cmds.h	17 May 2002 21:14:43 -0000
***************
*** 27,34 ****
  enum mi_cmd_result
    {
      /* Report the command as ``done''.  Display both the ``NNN^done''
!        message and the completion prompt.  */
!     MI_CMD_DONE = 0,
      /* The command is still running in the forground.  Main loop should
         display the completion prompt. */
      MI_CMD_FORGROUND,
--- 27,36 ----
  enum mi_cmd_result
    {
      /* Report the command as ``done''.  Display both the ``NNN^done''
!        message and the completion prompt.  Start this at 1 so that
!        you can distinguish ALL these returns from the 0 of an error
!        in catch_errors. */
!     MI_CMD_DONE = 1,
      /* The command is still running in the forground.  Main loop should
         display the completion prompt. */
      MI_CMD_FORGROUND,

Re: [RFC/MI] Renumber enum mi_cmd_result

[Andrew Cagney]

> Hi,
> 
> I've been looking over Apple's MI changes, so expect to see quite a few
> questions/changes/patches/debates/documentation updates/etc in the coming
> weeks on MI.
> 
> This simple patch renumbers enum mi_cmd_result so that it starts at one
> instead of zero. This is necessary because MI_CMD_DONE would be
> indistinguishable from an error condition (both would return zero to
> catch_errors).
> 
> I have run the testsuite before and after this change, and it produces no
> differences.
> 
> (Okay, this is almost too simple to leave by itself, but, gotta start
> somewhere!)
> 
> Keith
> 
> ChangeLog
> 2002-05-17  Keith Seitz  <keiths@redhat.com>
> 
>         From Jim Ingham <jingham at apple dot com>:
>         * mi-cmds.h (enum mi_cmd_result): Start numbering at one instead
>         of zero so that errors can be distinguished from other status
>         returns.
> 
Keith,

Check catch_exceptions().  The whole catch_errors() argument problem has 
been fixed.  I'd consider a switch to catch_exceptions obvious and I can 
see only one call.

Andrew

Re: [RFC/MI] Renumber enum mi_cmd_result

[Keith Seitz]

On Fri, 17 May 2002, Andrew Cagney wrote:

> Check catch_exceptions().  The whole catch_errors() argument problem has
> been fixed.  I'd consider a switch to catch_exceptions obvious and I can
> see only one call.

I thought you might say that! I'm already rewriting it...

(Just trying to keep you on your toes. :-)
Keith

Re: [RFC/MI] Renumber enum mi_cmd_result

[Keith Seitz]

On Fri, 17 May 2002, Andrew Cagney wrote:

> Check catch_exceptions().  The whole catch_errors() argument problem has
> been fixed.  I'd consider a switch to catch_exceptions obvious and I can
> see only one call.

Ugh. This turned out to be more elusive than I thought. After trying this
and that to untangle captured_mi_execute_command and catch_errors
(mi_execute_command used a return code that was a hybrid of both of
these), I've settled on the following patch, which at least attempts to
keep the mi return result of the command separate from what
mi_execute_command should do.

I've tested this on linux native and it causes no regressions.

Keith

PS. I don't consider this really obvious (even though the change is a
no-op).

mi/ChangeLog
2002-05-20  Keith Seitz  <keiths@redhat.com>

        * mi-main.c (captured_mi_execute_command): Add uiout parameter.
        "data" is now a structure which is used to pass data to/from this
        function to mi_execute_command.
        Modify function to comply with requirements from catch_exceptions.
        Store real return result and command's return result in data.
        (mi_execute_command): Use catch_exceptions.
        Use enum to handle actions to be performed instead of overloading
        catch_errors return result and the mi return result.

Patch
Index: mi/mi-main.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-main.c,v
retrieving revision 1.29
diff -p -r1.29 mi-main.c
*** mi/mi-main.c	21 Apr 2002 20:23:34 -0000	1.29
--- mi/mi-main.c	20 May 2002 17:05:45 -0000
*************** enum
*** 47,52 ****
--- 47,75 ----
      FROM_TTY = 0
    };

+ /* Enumerations of the actions that may result from calling
+    captured_mi_execute_command */
+
+ enum captured_mi_execute_command_actions
+   {
+     EXECUTE_COMMAND_DISPLAY_PROMPT,
+     EXECUTE_COMMAND_SUPRESS_PROMPT,
+     EXECUTE_COMMAND_DISPLAY_ERROR
+   };
+
+ /* This structure is used to pass information from captured_mi_execute_command
+    to mi_execute_command. */
+ struct captured_mi_execute_command_args
+ {
+   /* This return result of the MI command (output) */
+   enum mi_cmd_result rc;
+
+   /* What action to perform when the call is finished (output) */
+   enum captured_mi_execute_command_actions action;
+
+   /* The command context to be executed (input) */
+   struct mi_parse *command;
+ };

  int mi_debug_p;
  struct ui_file *raw_stdout;
*************** mi_cmd_data_write_memory (char *command,
*** 1025,1039 ****
    return MI_CMD_DONE;
  }

! /* Execute a command within a safe environment.  Return >0 for
!    ok. Return <0 for supress prompt.  Return 0 to have the error
!    extracted from error_last_message(). */

  static int
! captured_mi_execute_command (void *data)
  {
!   struct mi_parse *context = data;
!   enum mi_cmd_result rc;

    switch (context->op)
      {
--- 1048,1066 ----
    return MI_CMD_DONE;
  }

! /* Execute a command within a safe environment.
!    Return <0 for error; >=0 for ok.
!
!    args->action will tell mi_execute_command what action
!    to perfrom after the given command has executed (display/supress
!    prompt, display error). */

  static int
! captured_mi_execute_command (struct ui_out *uiout, void *data)
  {
!   struct captured_mi_execute_command_args *args =
!     (struct captured_mi_execute_command_args *) data;
!   struct mi_parse *context = args->command;

    switch (context->op)
      {
*************** captured_mi_execute_command (void *data)
*** 1048,1058 ****
           condition expression, each function should return an
           indication of what action is required and then switch on
           that. */
!       rc = mi_cmd_execute (context);
        if (!target_can_async_p () || !target_executing)
  	{
  	  /* print the result if there were no errors */
! 	  if (rc == MI_CMD_DONE)
  	    {
  	      fputs_unfiltered (context->token, raw_stdout);
  	      fputs_unfiltered ("^done", raw_stdout);
--- 1075,1087 ----
           condition expression, each function should return an
           indication of what action is required and then switch on
           that. */
!       args->action = EXECUTE_COMMAND_DISPLAY_PROMPT;
!       args->rc = mi_cmd_execute (context);
!
        if (!target_can_async_p () || !target_executing)
  	{
  	  /* print the result if there were no errors */
! 	  if (args->rc == MI_CMD_DONE)
  	    {
  	      fputs_unfiltered (context->token, raw_stdout);
  	      fputs_unfiltered ("^done", raw_stdout);
*************** captured_mi_execute_command (void *data)
*** 1060,1066 ****
  	      mi_out_rewind (uiout);
  	      fputs_unfiltered ("\n", raw_stdout);
  	    }
! 	  else if (rc == MI_CMD_ERROR)
  	    {
  	      if (mi_error_message)
  		{
--- 1089,1095 ----
  	      mi_out_rewind (uiout);
  	      fputs_unfiltered ("\n", raw_stdout);
  	    }
! 	  else if (args->rc == MI_CMD_ERROR)
  	    {
  	      if (mi_error_message)
  		{
*************** captured_mi_execute_command (void *data)
*** 1072,1089 ****
  		}
  	      mi_out_rewind (uiout);
  	    }
! 	  else if (rc == MI_CMD_CAUGHT_ERROR)
  	    {
  	      mi_out_rewind (uiout);
! 	      return 0;
  	    }
  	  else
  	    mi_out_rewind (uiout);
  	}
        else if (sync_execution)
! 	/* Don't print the prompt. We are executing the target in
! 	   synchronous mode. */
! 	return -1;
        break;

      case CLI_COMMAND:
--- 1101,1122 ----
  		}
  	      mi_out_rewind (uiout);
  	    }
! 	  else if (args->rc == MI_CMD_CAUGHT_ERROR)
  	    {
  	      mi_out_rewind (uiout);
! 	      args->action = EXECUTE_COMMAND_DISPLAY_ERROR;
! 	      return 1;
  	    }
  	  else
  	    mi_out_rewind (uiout);
  	}
        else if (sync_execution)
! 	{
! 	  /* Don't print the prompt. We are executing the target in
! 	     synchronous mode. */
! 	  args->action = EXECUTE_COMMAND_SUPRESS_PROMPT;
! 	  return 1;
! 	}
        break;

      case CLI_COMMAND:
*************** captured_mi_execute_command (void *data)
*** 1102,1110 ****
--- 1135,1146 ----
        mi_out_put (uiout, raw_stdout);
        mi_out_rewind (uiout);
        fputs_unfiltered ("\n", raw_stdout);
+       args->action = EXECUTE_COMMAND_DISPLAY_PROMPT;
+       args->rc = MI_CMD_DONE;
        break;

      }
+
    return 1;
  }

*************** void
*** 1113,1118 ****
--- 1149,1157 ----
  mi_execute_command (char *cmd, int from_tty)
  {
    struct mi_parse *command;
+   struct captured_mi_execute_command_args args;
+   struct ui_out *saved_uiout = uiout;
+   int result, rc;

    /* This is to handle EOF (^D). We just quit gdb. */
    /* FIXME: we should call some API function here. */
*************** mi_execute_command (char *cmd, int from_
*** 1123,1140 ****

    if (command != NULL)
      {
!       /* FIXME: cagney/1999-11-04: Can this use of catch_errors either
           be pushed even further down or even eliminated? */
!       int rc = catch_errors (captured_mi_execute_command, command, "",
! 			     RETURN_MASK_ALL);
!       if (rc < 0)
  	{
  	  /* The command is executing synchronously.  Bail out early
  	     suppressing the finished prompt. */
  	  mi_parse_free (command);
  	  return;
  	}
!       if (rc == 0)
  	{
  	  char *msg = error_last_message ();
  	  struct cleanup *cleanup = make_cleanup (xfree, msg);
--- 1162,1181 ----

    if (command != NULL)
      {
!       /* FIXME: cagney/1999-11-04: Can this use of catch_exceptions either
           be pushed even further down or even eliminated? */
!       args.command = command;
!       result = catch_exceptions (uiout, captured_mi_execute_command, &args, "",
! 				 RETURN_MASK_ALL);
!
!       if (args.action == EXECUTE_COMMAND_SUPRESS_PROMPT)
  	{
  	  /* The command is executing synchronously.  Bail out early
  	     suppressing the finished prompt. */
  	  mi_parse_free (command);
  	  return;
  	}
!       if (args.action == EXECUTE_COMMAND_DISPLAY_ERROR || result < 0)
  	{
  	  char *msg = error_last_message ();
  	  struct cleanup *cleanup = make_cleanup (xfree, msg);

Re: [RFC/MI] Renumber enum mi_cmd_result

[Andrew Cagney]

> 
> Ugh. This turned out to be more elusive than I thought. After trying this
> and that to untangle captured_mi_execute_command and catch_errors
> (mi_execute_command used a return code that was a hybrid of both of
> these), I've settled on the following patch, which at least attempts to
> keep the mi return result of the command separate from what
> mi_execute_command should do.
> 

Looks like a good move.  Yes.

Andrew

PS: catch_exceptions() is something of a compromise so I'm not suprized. 
  One early prototype had the wrapped function returning void so code 
was forced to pass the result back via the argument.

Re: [RFC/MI] Renumber enum mi_cmd_result

[Keith Seitz]

On Mon, 20 May 2002, Andrew Cagney wrote:

> Looks like a good move.  Yes.

Committed. Thanks for the ultra-quick review!

Keith