This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC/A] gdb/680: ui_out_reset?


On Sep 10,  4:44pm, Keith Seitz wrote:

> Following up to gdb/680 where the MI register commands cause assertion 
> failures with errors...
> 
> There are several ways to approach this. One is to make sure that 
> everytime ui_out_{list,tuple}_begin is called, there is a 
> ui_out_{list,tuple}_end. This leads to code like (from 
> mi_data_list_register_values):
> 
>   ui_out_list_begin (uiout, "register-values");
> 
>   if (argc == 1)		/* No args, beside the format: do all the regs */
>     {
>       for (regnum = 0;
> 	   regnum < numregs;
> 	   regnum++)
> 	{
> 	  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)
> 	    {
> 	      ui_out_tuple_end (uiout);
> 	      ui_out_list_end (uiout);
> 	      return MI_CMD_ERROR;
> 	    }
> 	  ui_out_tuple_end (uiout);
> 	}
>     }
>     /* snip */
> 
> I'd rather just propose a new ui-out function, ui_out_reset, which can be 
> called to "cleanup" the uiout in case of errors. In this case, 
> mi_out_rewind could call ui_out_reset and reset the "levels" field.
> 
> Of course, I think that Kevin also proposed something in the longjmp case 
> which is needed in these functions, afaict, but that's another bug/patch.
> 
> What do people think? Explicitly call ui_out_{list,tuple}_end before 
> returning an error code or add ui_out_reset? Perhaps another solution I've 
> overlooked?

Well, ui_out_reset() certainly seems easier, but I wonder if it's too
sloppy?  I.e, if we're going to do this, I'm wonder what the point of
checking the value of the ``level'' field (in push_level() and
pop_level()) in the first place is?

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.

So...  I guess I'm favoring explicit calls to ui_out_{list,tuple}_end().

Kevin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]