This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFC/A] gdb/680: ui_out_reset?
- From: Keith Seitz <keiths at redhat dot com>
- To: gdb-patches at sources dot redhat dot com
- Date: Wed, 11 Sep 2002 12:15:28 -0700 (PDT)
- Subject: Re: [RFC/A] gdb/680: ui_out_reset?
On Tue, 10 Sep 2002, Kevin Buettner wrote:
> Hmm. I guess it prevents you from writing code which goes too deep.
> But it doesn't really help you to check to see if you've correctly
> popped each level. At the moment, if you don't have enough pops,
> you'll eventually wind up seeing an internal error. (Which is a good
> thing, I think.)
>
> It also occurs to me that making sure that you've done the pops in the
> right places is important when the caller (of a function which
> returns MI_CMD_ERROR) decides to do something other than return to
> the top level. I.e, maybe it decides that it can somehow continue and
> produces more output.
Wow, really good points. I hadn't thought of this last one, but I still
wish there was something a little less error-prone than remembering to pop
levels off of the uiout. Although we're lucky that this has only happened
in one isolated area of MI, it seems to me that this sort of thing could
creep in elsewhere, because it's really not very obvious or logical, I
think.
I'll assume that it is a goal to make programming MI as much like
programming gdb as possible. Then it follows that what we should be doing
is using cleanups.
In this case, the register commands (and probably a lot of other commands,
too) are just plain programmed wrong. They should be using
make_cleanup_ui_out_list/tuple_begin_end instead of
ui_out_list/tuple_begin and ui_out_list/tuple_end.
How's this patch look? It doesn't cause an regressions and fixes gdb/680.
Keith
ChangeLog
2002-09-11 Keith Seitz <keiths@redhat.com>
* mi-main.c (mi_cmd_data_list_register_names): Use cleanups
for the uiout list. Do the cleanups when returning an error.
(mi_cmd_data_list_changed_registers): Ditto.
(mi_cmd_data_list_register_values): Use cleanups for the uiout list
and tuples. Do the cleanups when returning errors.
Patch
Index: mi/mi-main.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-main.c,v
retrieving revision 1.30
diff -p -r1.30 mi-main.c
*** mi/mi-main.c 20 May 2002 18:09:57 -0000 1.30
--- mi/mi-main.c 11 Sep 2002 19:02:49 -0000
*************** mi_cmd_data_list_register_names (char *c
*** 275,280 ****
--- 275,281 ----
{
int regnum, numregs;
int i;
+ struct cleanup *cleanup;
/* Note that the test for a valid register must include checking the
REGISTER_NAME because NUM_REGS may be allocated for the union of
*************** mi_cmd_data_list_register_names (char *c
*** 284,290 ****
numregs = NUM_REGS + NUM_PSEUDO_REGS;
! ui_out_list_begin (uiout, "register-names");
if (argc == 0) /* No args, just do all the regs */
{
--- 285,291 ----
numregs = NUM_REGS + NUM_PSEUDO_REGS;
! cleanup = make_cleanup_ui_out_list_begin_end (uiout, "register-names");
if (argc == 0) /* No args, just do all the regs */
{
*************** mi_cmd_data_list_register_names (char *c
*** 306,311 ****
--- 307,313 ----
regnum = atoi (argv[i]);
if (regnum < 0 || regnum >= numregs)
{
+ do_cleanups (cleanup);
xasprintf (&mi_error_message, "bad register number");
return MI_CMD_ERROR;
}
*************** mi_cmd_data_list_register_names (char *c
*** 315,321 ****
else
ui_out_field_string (uiout, NULL, REGISTER_NAME (regnum));
}
! ui_out_list_end (uiout);
return MI_CMD_DONE;
}
--- 317,323 ----
else
ui_out_field_string (uiout, NULL, REGISTER_NAME (regnum));
}
! do_cleanups (cleanup);
return MI_CMD_DONE;
}
*************** mi_cmd_data_list_changed_registers (char
*** 324,329 ****
--- 326,332 ----
{
int regnum, numregs, changed;
int i;
+ struct cleanup *cleanup;
/* Note that the test for a valid register must include checking the
REGISTER_NAME because NUM_REGS may be allocated for the union of
*************** mi_cmd_data_list_changed_registers (char
*** 333,339 ****
numregs = NUM_REGS;
! ui_out_list_begin (uiout, "changed-registers");
if (argc == 0) /* No args, just do all the regs */
{
--- 336,342 ----
numregs = NUM_REGS;
! cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changed-registers");
if (argc == 0) /* No args, just do all the regs */
{
*************** mi_cmd_data_list_changed_registers (char
*** 347,352 ****
--- 350,356 ----
changed = register_changed_p (regnum);
if (changed < 0)
{
+ do_cleanups (cleanup);
xasprintf (&mi_error_message,
"mi_cmd_data_list_changed_registers: Unable to read register contents.");
return MI_CMD_ERROR;
*************** mi_cmd_data_list_changed_registers (char
*** 369,374 ****
--- 373,379 ----
changed = register_changed_p (regnum);
if (changed < 0)
{
+ do_cleanups (cleanup);
xasprintf (&mi_error_message,
"mi_cmd_data_list_register_change: Unable to read register contents.");
return MI_CMD_ERROR;
*************** mi_cmd_data_list_changed_registers (char
*** 378,388 ****
}
else
{
xasprintf (&mi_error_message, "bad register number");
return MI_CMD_ERROR;
}
}
! ui_out_list_end (uiout);
return MI_CMD_DONE;
}
--- 383,394 ----
}
else
{
+ do_cleanups (cleanup);
xasprintf (&mi_error_message, "bad register number");
return MI_CMD_ERROR;
}
}
! do_cleanups (cleanup);
return MI_CMD_DONE;
}
*************** mi_cmd_data_list_register_values (char *
*** 418,423 ****
--- 424,430 ----
{
int regnum, numregs, format, result;
int i;
+ struct cleanup *list_cleanup, *tuple_cleanup;
/* Note that the test for a valid register must include checking the
REGISTER_NAME because NUM_REGS may be allocated for the union of
*************** mi_cmd_data_list_register_values (char *
*** 443,449 ****
return MI_CMD_ERROR;
}
! ui_out_list_begin (uiout, "register-values");
if (argc == 1) /* No args, beside the format: do all the regs */
{
--- 450,456 ----
return MI_CMD_ERROR;
}
! list_cleanup = make_cleanup_ui_out_list_begin_end (uiout, "register-values");
if (argc == 1) /* No args, beside the format: do all the regs */
{
*************** mi_cmd_data_list_register_values (char *
*** 454,465 ****
if (REGISTER_NAME (regnum) == NULL
|| *(REGISTER_NAME (regnum)) == '\0')
continue;
! ui_out_tuple_begin (uiout, NULL);
ui_out_field_int (uiout, "number", regnum);
result = get_register (regnum, format);
if (result == -1)
! return MI_CMD_ERROR;
! ui_out_tuple_end (uiout);
}
}
--- 461,475 ----
if (REGISTER_NAME (regnum) == NULL
|| *(REGISTER_NAME (regnum)) == '\0')
continue;
! tuple_cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
ui_out_field_int (uiout, "number", regnum);
result = get_register (regnum, format);
if (result == -1)
! {
! do_cleanups (list_cleanup);
! return MI_CMD_ERROR;
! }
! do_cleanups (tuple_cleanup);
}
}
*************** mi_cmd_data_list_register_values (char *
*** 473,492 ****
&& REGISTER_NAME (regnum) != NULL
&& *REGISTER_NAME (regnum) != '\000')
{
! ui_out_tuple_begin (uiout, NULL);
ui_out_field_int (uiout, "number", regnum);
result = get_register (regnum, format);
if (result == -1)
! return MI_CMD_ERROR;
! ui_out_tuple_end (uiout);
}
else
{
xasprintf (&mi_error_message, "bad register number");
return MI_CMD_ERROR;
}
}
! ui_out_list_end (uiout);
return MI_CMD_DONE;
}
--- 483,506 ----
&& REGISTER_NAME (regnum) != NULL
&& *REGISTER_NAME (regnum) != '\000')
{
! tuple_cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
ui_out_field_int (uiout, "number", regnum);
result = get_register (regnum, format);
if (result == -1)
! {
! do_cleanups (list_cleanup);
! return MI_CMD_ERROR;
! }
! do_cleanups (tuple_cleanup);
}
else
{
+ do_cleanups (list_cleanup);
xasprintf (&mi_error_message, "bad register number");
return MI_CMD_ERROR;
}
}
! do_cleanups (list_cleanup);
return MI_CMD_DONE;
}