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

Nick Roberts
2007-03-22T04:35:58Z

> 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 ----