[RFC] gdb_breakpoint Re: [RFC] -thread-info new command

[Nick Roberts <nickrob@snap.net.nz>]

 > Actually there are currently two ways to catch an error in MI:
 > 
 > 1) Using error () and catch_exception.
 > 
 > 2) Using MI_CMD_ERROR and mi_error_message.
 > 
 > The first gets caught in mi_execute_command and the error message is stored
 > in result.message.  The second goes back to captured_mi_execute_command and
 > the error message is manually stored in mi_error_message.
 > 
 > I think that only one method should be used and this should be the first one.

Here's a patch which merges do_captured_breakpoint into gdb_breakpoint so that
errors get caught further up in mi_execute_command.  The same thing can be done
for gdb_breakpoint_query and gdb_list_thread_ids.  The function
gdb_thread_select is also used by CLI, but I don't know if that would
cause problems.

gdb_breakpoint has the extra complication that deprecated_set_gdb_event_hooks
have to be restored.  I've tried to deal with that in the changes to
mi-cmd-break.c.

This is a request for comment, not approval.  I guess cleanup_gdb_event_hooks
would have to go elsewhere/have another name.  There are probably other issues.
I'm trying to float ideas.

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


2007-03-22  Nick Roberts  <nickrob@snap.net.nz>

	* mi/mi-cmd-break.c (cleanup_gdb_event_hooks): New function.
	(mi_cmd_break_insert): Use it.

	* breakpoint.c (captured_breakpoint_args): Delete.
	(do_captured_breakpoint): Delete.  Merge functionality into...
	(gdb_breakpoint): ...here.  Don't catch errors.


*** mi-cmd-break.c	10 Jan 2007 11:56:57 +1300	1.15
--- mi-cmd-break.c	22 Mar 2007 16:19:17 +1200	
*************** enum bp_type
*** 58,63 ****
--- 58,69 ----
      REGEXP_BP
    };
  
+ void
+ cleanup_gdb_event_hooks (void *ptr)
+ {
+   deprecated_set_gdb_event_hooks (ptr);
+ }
+ 
  /* Insert a breakpoint. The type of breakpoint is specified by the
     first argument: -break-insert <location> --> insert a regular
     breakpoint.  -break-insert -t <location> --> insert a temporary
*************** mi_cmd_break_insert (char *command, char
*** 77,82 ****
--- 83,89 ----
    int ignore_count = 0;
    char *condition = NULL;
    enum gdb_rc rc;
+   struct cleanup *old_cleanups;
    struct gdb_events *old_hooks;
    enum opt
      {
*************** mi_cmd_break_insert (char *command, char
*** 135,153 ****
  
    /* Now we have what we need, let's insert the breakpoint! */
    old_hooks = deprecated_set_gdb_event_hooks (&breakpoint_hooks);
    switch (type)
      {
      case REG_BP:
        rc = gdb_breakpoint (address, condition,
  			   0 /*hardwareflag */ , temp_p,
! 			   thread, ignore_count,
! 			   &mi_error_message);
        break;
      case HW_BP:
        rc = gdb_breakpoint (address, condition,
  			   1 /*hardwareflag */ , temp_p,
! 			   thread, ignore_count,
! 			   &mi_error_message);
        break;
  #if 0
      case REGEXP_BP:
--- 142,159 ----
  
    /* Now we have what we need, let's insert the breakpoint! */
    old_hooks = deprecated_set_gdb_event_hooks (&breakpoint_hooks);
+   old_cleanups = make_cleanup (cleanup_gdb_event_hooks, old_hooks);
    switch (type)
      {
      case REG_BP:
        rc = gdb_breakpoint (address, condition,
  			   0 /*hardwareflag */ , temp_p,
! 			   thread, ignore_count);
        break;
      case HW_BP:
        rc = gdb_breakpoint (address, condition,
  			   1 /*hardwareflag */ , temp_p,
! 			   thread, ignore_count);
        break;
  #if 0
      case REGEXP_BP:
*************** mi_cmd_break_insert (char *command, char
*** 162,168 ****
        internal_error (__FILE__, __LINE__,
  		      _("mi_cmd_break_insert: Bad switch."));
      }
!   deprecated_set_gdb_event_hooks (old_hooks);
  
    if (rc == GDB_RC_FAIL)
      return MI_CMD_ERROR;
--- 168,174 ----
        internal_error (__FILE__, __LINE__,
  		      _("mi_cmd_break_insert: Bad switch."));
      }
!   do_cleanups (old_cleanups);
  
    if (rc == GDB_RC_FAIL)
      return MI_CMD_ERROR;


*** breakpoint.c	14 Mar 2007 00:13:43 +1300	1.242
--- breakpoint.c	22 Mar 2007 12:57:35 +1200	
*************** break_command_1 (char *arg, int flag, in
*** 5476,5498 ****
    return GDB_RC_OK;
  }
  
! /* Set a breakpoint of TYPE/DISPOSITION according to ARG (function,
!    linenum or *address) with COND and IGNORE_COUNT. */
  
! struct captured_breakpoint_args
!   {
!     char *address;
!     char *condition;
!     int hardwareflag;
!     int tempflag;
!     int thread;
!     int ignore_count;
!   };
! 
! static int
! do_captured_breakpoint (struct ui_out *uiout, void *data)
  {
-   struct captured_breakpoint_args *args = data;
    struct symtabs_and_lines sals;
    struct expression **cond;
    struct cleanup *old_chain;
--- 5476,5489 ----
    return GDB_RC_OK;
  }
  
! /* Set a breakpoint of TYPE/DISPOSITION according to function,
!    linenum or *address, with COND and IGNORE_COUNT. */
  
! enum gdb_rc
! gdb_breakpoint (char *address, char *condition,
! 		int hardwareflag, int tempflag,
! 		int thread, int ignore_count)
  {
    struct symtabs_and_lines sals;
    struct expression **cond;
    struct cleanup *old_chain;
*************** do_captured_breakpoint (struct ui_out *u
*** 5508,5514 ****
       place. */
    sals.sals = NULL;
    sals.nelts = 0;
!   address_end = args->address;
    addr_string = NULL;
    parse_breakpoint_sals (&address_end, &sals, &addr_string, 0);
  
--- 5499,5505 ----
       place. */
    sals.sals = NULL;
    sals.nelts = 0;
!   address_end = address;
    addr_string = NULL;
    parse_breakpoint_sals (&address_end, &sals, &addr_string, 0);
  
*************** do_captured_breakpoint (struct ui_out *u
*** 5554,5580 ****
      error (_("Garbage %s following breakpoint address"), address_end);
  
    /* Resolve all line numbers to PC's.  */
!   breakpoint_sals_to_pc (&sals, args->address);
  
    /* Verify that conditions can be parsed, before setting any
       breakpoints.  */
    for (i = 0; i < sals.nelts; i++)
      {
!       if (args->condition != NULL)
  	{
! 	  char *tok = args->condition;
  	  cond[i] = parse_exp_1 (&tok, block_for_pc (sals.sals[i].pc), 0);
  	  if (*tok != '\0')
  	    error (_("Garbage %s follows condition"), tok);
  	  make_cleanup (xfree, cond[i]);
! 	  cond_string[i] = xstrdup (args->condition);
  	}
      }
  
    create_breakpoints (sals, addr_string, cond, cond_string,
! 		      args->hardwareflag ? bp_hardware_breakpoint : bp_breakpoint,
! 		      args->tempflag ? disp_del : disp_donttouch,
! 		      args->thread, args->ignore_count, 0/*from-tty*/, 
  		      NULL/*pending_bp*/);
  
    /* That's it. Discard the cleanups for data inserted into the
--- 5545,5571 ----
      error (_("Garbage %s following breakpoint address"), address_end);
  
    /* Resolve all line numbers to PC's.  */
!   breakpoint_sals_to_pc (&sals, address);
  
    /* Verify that conditions can be parsed, before setting any
       breakpoints.  */
    for (i = 0; i < sals.nelts; i++)
      {
!       if (condition != NULL)
  	{
! 	  char *tok = condition;
  	  cond[i] = parse_exp_1 (&tok, block_for_pc (sals.sals[i].pc), 0);
  	  if (*tok != '\0')
  	    error (_("Garbage %s follows condition"), tok);
  	  make_cleanup (xfree, cond[i]);
! 	  cond_string[i] = xstrdup (condition);
  	}
      }
  
    create_breakpoints (sals, addr_string, cond, cond_string,
! 		      hardwareflag ? bp_hardware_breakpoint : bp_breakpoint,
! 		      tempflag ? disp_del : disp_donttouch,
! 		      thread, ignore_count, 0/*from-tty*/, 
  		      NULL/*pending_bp*/);
  
    /* That's it. Discard the cleanups for data inserted into the
*************** do_captured_breakpoint (struct ui_out *u
*** 5585,5608 ****
    return GDB_RC_OK;
  }
  
- enum gdb_rc
- gdb_breakpoint (char *address, char *condition,
- 		int hardwareflag, int tempflag,
- 		int thread, int ignore_count,
- 		char **error_message)
- {
-   struct captured_breakpoint_args args;
-   args.address = address;
-   args.condition = condition;
-   args.hardwareflag = hardwareflag;
-   args.tempflag = tempflag;
-   args.thread = thread;
-   args.ignore_count = ignore_count;
-   return catch_exceptions_with_msg (uiout, do_captured_breakpoint, &args,
- 				    error_message, RETURN_MASK_ALL);
- }
- 
- 
  /* Helper function for break_command_1 and disassemble_command.  */
  
  void
--- 5576,5581 ----