Re: asynchronous MI output commands

[Bob Rossi <bob_rossi@cox.net>]

On Thu, May 11, 2006 at 03:23:22PM +0400, Vladimir Prus wrote:
> On Thursday 11 May 2006 14:55, Bob Rossi wrote:
> 
> > > > >     const GDBMI::Value& children = r["children"];
> > > > >
> > > > >     for (unsigned i = 0; i < children.size(); ++i)
> > > > >     {
> > > > >         QString exp = children[i]["exp"].literal();
> > > > >
> > > > >
> > > > > If you have specific structures for each response this won't be very
> > > > > much simpler.
> > > >
> > > > Sorry, I've described this before, but apparently not good enough. I
> > > > definatly can create the abstract parse tree with out knowing the input
> > > > command. However, then I want to create C data structures for each
> > > > MI output.
> > >
> > > Why? With C data structures, the above frontend code will be only
> > > marginally simpler.
> >
> > I don't believe that to be the case at all. Look at the C data structure
> > code I have for the -break-list data structure. If a GUI had to use this I
> > think they would be happy.
> 
> So, instead of say:
> 
>     bp["number"].toInt()
> 
> I'd write:
> 
>     bp.number
> 
> ? Well, that's what I call "marginally better". It's better, but not likely to 
> make big practical difference.
> 
> Also, you propose C interface, so is not directly usable in frontend written 
> in C++. Such frontend will likely to have it's own class for breakpoint, and 
> would have to convert from your structure to that class.

Yeah, this will have to happen regardless. I mean, if I only have the
parse tree, it will need to be ported to other languages also. For C++,
it's particular easy, you can either inherit from the struct or use
composition.

> I guess if you want to make MI parser for your own use, any approach is fine. 
> But if you want to make some kind of "standard" MI parser for other frontends 
> to use, I'm not sure that providing such pre-made structures will be a 
> selling point.

Well, I suppose I'll provide them for myself, and others can easily use
the parse tree if that's what they want to do. However, I must warn you,
using the parse tree is more complicated than what you have above. I'm
not exactly sure how you can simply say 
  bp["number"].toInt ()
where does "bp" come from? Is it a pointer into the parse tree? If so,
how did you get to that point?

I have to get the parse tree, which is stored in 'output_ptr' below.
Then i have to get all the way to the result_ptr which has "number" as
the variable. I see you are using a map in C++ which makes the code much
cleaner. I don't have that advantage in C.
  
    gdbmi_result_ptr result_ptr = output_ptr->result_record->result;
    if (strcmp (result_ptr->variable, "BreakpointTable") == 0)
    {
      if (result_ptr->value->value_choice == GDBMI_TUPLE)
      {
	result_ptr = result_ptr->value->option.tuple->result;
	while (result_ptr)
	{
	  if (strcmp (result_ptr->variable, "body") == 0)
	  {
	    if (result_ptr->value->value_choice == GDBMI_LIST)
	    {
	      gdbmi_list_ptr list_ptr = result_ptr->value->option.list;
	      if (list_ptr && list_ptr->list_choice == GDBMI_RESULT)
	      {
		gdbmi_result_ptr result_ptr = list_ptr->option.result;
		while (result_ptr)
		{
		  if (strcmp (result_ptr->variable, "bkpt") == 0)
		  {
		    gdbmi_oc_breakpoint_ptr ptr = create_gdbmi_breakpoint ();

		    gdbmi_value_ptr value_ptr = result_ptr->value;
		    if (value_ptr->value_choice == GDBMI_TUPLE)
		    {
		      gdbmi_result_ptr result_ptr = value_ptr->option.tuple->result;
		      while (result_ptr)
		      {
			if (strcmp (result_ptr->variable, "number") == 0)
			{
			  if (result_ptr->value->value_choice == GDBMI_CSTRING)
			  {
			    char *nstr;
			    if (convert_cstring (result_ptr->value->option.cstring, &nstr) == -1)
			    {
			      fprintf (stderr, "%s:%d\n", __FILE__, __LINE__);
			      return -1;
			    }

			    ptr->number = atoi (nstr);
			    free (nstr);
			    nstr = NULL;
			  }
			} 
			result_ptr = result_ptr->next;
		      }
		    }

		    oc_ptr->input_commands.break_list.breakpoint_ptr =
		       append_gdbmi_breakpoint (oc_ptr->input_commands.break_list.breakpoint_ptr, ptr);
		  }
		  result_ptr = result_ptr->next;
		}
	      }
	    }
	  }
	  result_ptr = result_ptr->next;
	}
      }
    }

> > I do 
> > know that I have some code duplicated that could be in a function, and
> > some other cleanups, but I still think it's far more difficult.
> 
> I think the code you've presented indeed has too much copy-paste. For example:
> 
>  			else if (strcmp (result_ptr->variable, "addr") == 0)
>  			{
>  			  if (result_ptr->value->value_choice == GDBMI_CSTRING)
>  			  {
>  			    if (convert_cstring (result_ptr->value->option.cstring,
>                       &ptr->address) == -1) 
>                   {
>  			      fprintf (stderr, "%s:%d\n", __FILE__, __LINE__);
>  			      return -1;
>  			    }
>  			  }
>  			}
> 
> is a pattern that repeats for many of fields. While in my interface, you'd 
> call:
> 
>    bp["addr"].literal()
> 
> to get exactly same effect. Except that in your code, no error is emitted when 
> "addr" field is not of string type ;-)

I intentionally left out the error. I'm not sure how much error code I
should have in this library if it needs to work in all circumstances.
I'm still thinking about that.

> Maybe if you provide functions  'get_literal' and 'get_integer' and 
> 'get_bool_yn', the size of code can be reduced considerably.

Thanks for the suggestion, I'll try that.

Bob Rossi