Re: [RFC] varobj deletion after the binary has changed

[Nick Roberts <nickrob at snap dot net dot nz>]

 > Attached is a new implementation, it takes care of more cases where seg 
 > fault occured.
 > 
 > I take Nick's suggestion into account, -var-update outputs the following 
 > for invalid variables:
 >     changelist=[{name="linteger",in_scope="invalid"}]
 > 
 > About this last change, I'm wondering if frond-ends that expect "true" 
 > or "false" will handle this new outputs correctly, it's just a question 
 > of backward compatibility. Of course it will be better that the old 
 > behavior that crashes gdb !

The documentation doesn't say that "true" and "false" are exhaustive
possibilities but it may be reasonable to expect that they are.  It's only
needed when the inferior is rebuilt with extant variable objects.  I've never
found a problem in my use, although it clearly depends on pattern of use.
In any case, as you say, if we make no change gdb will just crash.

If Daniel approves the patch, I will submit some documentation as we are
considering other values for in_scope such as "exited".  That way frontend
developers can factor in the possobility of further values.

 > If you're fine I'll write appropriate ChangeLogs.
 > 
 > I also wrote a new .exp file in gdb.mi testsuite, I'll propose it on 
 > this one approval.
 
Looks good.  Maybe varobj_update could use an enum:

enum varobj_update_values
  {
    SCOPE_FALSE = -1,
    TYPE_CHANGED,
    SCOPE_INVALID
  }

 >...
 > Index: mi/mi-cmd-var.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/mi/mi-cmd-var.c,v
 > retrieving revision 1.28
 > diff -u -p -r1.28 mi-cmd-var.c
 > --- mi/mi-cmd-var.c	9 Jan 2007 17:59:08 -0000	1.28
 > +++ mi/mi-cmd-var.c	29 Jan 2007 12:09:58 -0000
 > @@ -555,12 +555,12 @@ varobj_update_one (struct varobj *var, e
 >    
 >    if (nc == 0)
 >      return 1;
 > -  else if (nc == -1)
 > +  else if (nc == -1 || nc == -3)
 >      {
 >        if (mi_version (uiout) > 1)
 >          cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 >        ui_out_field_string (uiout, "name", varobj_get_objname(var));
 > -      ui_out_field_string (uiout, "in_scope", "false");
 > +      ui_out_field_string (uiout, "in_scope", (nc == -3 ? "invalid" : "false"));
 >        if (mi_version (uiout) > 1)
 >          do_cleanups (cleanup);
 >        return -1;


Including the case nc == -2, maybe this reads more easily (just thinking out
aloud):

  else if (nc < 0)
    {
      if (mi_version (uiout) > 1)
        cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
      ui_out_field_string (uiout, "name", varobj_get_objname(var));
      switch (nc)
      {
        case (SCOPE_FALSE)
          ui_out_field_string (uiout, "in_scope", "false");
	  break;
        case (TYPE_CHANGED)
	  ui_out_field_string (uiout, "in_scope", "true");
          ui_out_field_string (uiout, "new_type", varobj_get_type(var));
          ui_out_field_int (uiout, "new_num_children", 
			    varobj_get_num_children(var));
	  break;
        case (SCOPE_invalid)
          ui_out_field_string (uiout, "in_scope", "invalid");
 	  break;
      }
      if (mi_version (uiout) > 1)
        do_cleanups (cleanup);
    }

(It looks like we could remove the return value of varobj_update_one as it
doesn't seem to be used.)


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