[RFA] varobj.c/h interface change to varobj_update

[RFA] varobj.c/h interface change to varobj_update

[Keith Seitz]

Hi,

I would like to change the interface for the function varobj_update from

int varobj_update (struct varobj *var, struct varobj ***changelist);

to

int varobj_update (struct varobj **varp, struct varobj ***changelist);

This is because varobj_update calls value_of_root, which can delete
varobjs from underneath the caller's nose. value_of_root already requires
a pointer to struct varobj* for this reason. I am simply propagating this
information up the food chain. Strictly not necessary (the function
returns -2 if this happens and callers can re-query the object), but I
feel this is much fool-proof and just plain makes more sense.
(And I threw in small bugfix, too.)

If this is approved, I will commit changes to mi/mi-cmd-var.c, which will
then be "obvious."

As always, I await your comments.
Keith

ChangeLog
2001-08-17  Keith Seitz  <keiths@redhat.com>

	* varobj.c (varobj_update): Change first parameter to
	pointer to struct varobj*. This function can delete
	varobjs, so we need to give callers the new varobj
	when this happens.
	(value_of_root): Update "var", too, if "var_handle"
	changes.
	* varobj.h (varobj_update): Likewise.

Patch
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.19
diff -u -p -r1.19 varobj.c
--- varobj.c	2001/05/10 16:57:01	1.19
+++ varobj.c	2001/08/17 18:13:48
@@ -866,10 +866,14 @@ varobj_list (struct varobj ***varlist)
     -2 if the type changed
     Otherwise it is the number of children + parent changed

-   Only root variables can be updated... */
+   Only root variables can be updated...

+   NOTE: This function may delete the caller's varobj. If it
+   returns -2, then it has done this and VARP will be modified
+   to point to the new varobj. */
+
 int
-varobj_update (struct varobj *var, struct varobj ***changelist)
+varobj_update (struct varobj **varp, struct varobj ***changelist)
 {
   int changed = 0;
   int type_changed;
@@ -889,7 +893,7 @@ varobj_update (struct varobj *var, struc
     return -1;

   /*  Only root variables can be updated... */
-  if (var->root->rootvar != var)
+  if ((*varp)->root->rootvar != *varp)
     /* Not a root var */
     return -1;

@@ -903,10 +907,10 @@ varobj_update (struct varobj *var, struc
      value_of_root variable dispose of the varobj if the type
      has changed. */
   type_changed = 1;
-  new = value_of_root (&var, &type_changed);
+  new = value_of_root (varp, &type_changed);
   if (new == NULL)
     {
-      var->error = 1;
+      (*varp)->error = 1;
       return -1;
     }

@@ -917,34 +921,34 @@ varobj_update (struct varobj *var, struc
      them note that it's changed. */
   if (type_changed)
     {
-      vpush (&result, var);
+      vpush (&result, *varp);
       changed++;
     }
   /* If values are not equal, note that it's changed.
      There a couple of exceptions here, though.
      We don't want some types to be reported as "changed". */
-  else if (type_changeable (var) && !my_value_equal (var->value, new, &error2))
+  else if (type_changeable (*varp) && !my_value_equal ((*varp)->value, new, &error2))
     {
-      vpush (&result, var);
+      vpush (&result, *varp);
       changed++;
       /* error2 replaces var->error since this new value
          WILL replace the old one. */
-      var->error = error2;
+      (*varp)->error = error2;
     }

   /* We must always keep around the new value for this root
      variable expression, or we lose the updated children! */
-  value_free (var->value);
-  var->value = new;
+  value_free ((*varp)->value);
+  (*varp)->value = new;

   /* Initialize a stack */
   vpush (&stack, NULL);

   /* Push the root's children */
-  if (var->children != NULL)
+  if ((*varp)->children != NULL)
     {
       struct varobj_child *c;
-      for (c = var->children; c != NULL; c = c->next)
+      for (c = (*varp)->children; c != NULL; c = c->next)
 	vpush (&stack, c->child);
     }

@@ -1647,6 +1651,7 @@ value_of_root (struct varobj **var_handl
 	    }
 	  install_variable (tmp_var);
 	  *var_handle = tmp_var;
+	  var = *var_handle;
 	  *type_changed = 1;
 	}
     }
Index: varobj.h
===================================================================
RCS file: /cvs/src/src/gdb/varobj.h,v
retrieving revision 1.3
diff -u -p -r1.3 varobj.h
--- varobj.h	2001/03/06 08:21:18	1.3
+++ varobj.h	2001/08/17 18:13:48
@@ -95,6 +95,6 @@ extern int varobj_set_value (struct varo

 extern int varobj_list (struct varobj ***rootlist);

-extern int varobj_update (struct varobj *var, struct varobj ***changelist);
+extern int varobj_update (struct varobj **varp, struct varobj ***changelist);

 #endif /* VAROBJ_H */

Re: [RFA] varobj.c/h interface change to varobj_update

[Fernando Nasser]

Keith Seitz wrote:
> 
> Hi,
> 
> I would like to change the interface for the function varobj_update from
> 
> int varobj_update (struct varobj *var, struct varobj ***changelist);
> 
> to
> 
> int varobj_update (struct varobj **varp, struct varobj ***changelist);
> 
> This is because varobj_update calls value_of_root, which can delete
> varobjs from underneath the caller's nose. value_of_root already requires
> a pointer to struct varobj* for this reason. I am simply propagating this
> information up the food chain. Strictly not necessary (the function
> returns -2 if this happens and callers can re-query the object), but I
> feel this is much fool-proof and just plain makes more sense.
> (And I threw in small bugfix, too.)
> 
> If this is approved, I will commit changes to mi/mi-cmd-var.c, which will
> then be "obvious."
> 
> As always, I await your comments.
> Keith
> 

It is OK with me.  I know that you will take care of things if something
breaks ;-)

Thanks.

Fernando




> ChangeLog
> 2001-08-17  Keith Seitz  <keiths@redhat.com>
> 
>         * varobj.c (varobj_update): Change first parameter to
>         pointer to struct varobj*. This function can delete
>         varobjs, so we need to give callers the new varobj
>         when this happens.
>         (value_of_root): Update "var", too, if "var_handle"
>         changes.
>         * varobj.h (varobj_update): Likewise.
> 
> Patch
> Index: varobj.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/varobj.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 varobj.c
> --- varobj.c    2001/05/10 16:57:01     1.19
> +++ varobj.c    2001/08/17 18:13:48
> @@ -866,10 +866,14 @@ varobj_list (struct varobj ***varlist)
>      -2 if the type changed
>      Otherwise it is the number of children + parent changed
> 
> -   Only root variables can be updated... */
> +   Only root variables can be updated...
> 
> +   NOTE: This function may delete the caller's varobj. If it
> +   returns -2, then it has done this and VARP will be modified
> +   to point to the new varobj. */
> +
>  int
> -varobj_update (struct varobj *var, struct varobj ***changelist)
> +varobj_update (struct varobj **varp, struct varobj ***changelist)
>  {
>    int changed = 0;
>    int type_changed;
> @@ -889,7 +893,7 @@ varobj_update (struct varobj *var, struc
>      return -1;
> 
>    /*  Only root variables can be updated... */
> -  if (var->root->rootvar != var)
> +  if ((*varp)->root->rootvar != *varp)
>      /* Not a root var */
>      return -1;
> 
> @@ -903,10 +907,10 @@ varobj_update (struct varobj *var, struc
>       value_of_root variable dispose of the varobj if the type
>       has changed. */
>    type_changed = 1;
> -  new = value_of_root (&var, &type_changed);
> +  new = value_of_root (varp, &type_changed);
>    if (new == NULL)
>      {
> -      var->error = 1;
> +      (*varp)->error = 1;
>        return -1;
>      }
> 
> @@ -917,34 +921,34 @@ varobj_update (struct varobj *var, struc
>       them note that it's changed. */
>    if (type_changed)
>      {
> -      vpush (&result, var);
> +      vpush (&result, *varp);
>        changed++;
>      }
>    /* If values are not equal, note that it's changed.
>       There a couple of exceptions here, though.
>       We don't want some types to be reported as "changed". */
> -  else if (type_changeable (var) && !my_value_equal (var->value, new, &error2))
> +  else if (type_changeable (*varp) && !my_value_equal ((*varp)->value, new, &error2))
>      {
> -      vpush (&result, var);
> +      vpush (&result, *varp);
>        changed++;
>        /* error2 replaces var->error since this new value
>           WILL replace the old one. */
> -      var->error = error2;
> +      (*varp)->error = error2;
>      }
> 
>    /* We must always keep around the new value for this root
>       variable expression, or we lose the updated children! */
> -  value_free (var->value);
> -  var->value = new;
> +  value_free ((*varp)->value);
> +  (*varp)->value = new;
> 
>    /* Initialize a stack */
>    vpush (&stack, NULL);
> 
>    /* Push the root's children */
> -  if (var->children != NULL)
> +  if ((*varp)->children != NULL)
>      {
>        struct varobj_child *c;
> -      for (c = var->children; c != NULL; c = c->next)
> +      for (c = (*varp)->children; c != NULL; c = c->next)
>         vpush (&stack, c->child);
>      }
> 
> @@ -1647,6 +1651,7 @@ value_of_root (struct varobj **var_handl
>             }
>           install_variable (tmp_var);
>           *var_handle = tmp_var;
> +         var = *var_handle;
>           *type_changed = 1;
>         }
>      }
> Index: varobj.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/varobj.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 varobj.h
> --- varobj.h    2001/03/06 08:21:18     1.3
> +++ varobj.h    2001/08/17 18:13:48
> @@ -95,6 +95,6 @@ extern int varobj_set_value (struct varo
> 
>  extern int varobj_list (struct varobj ***rootlist);
> 
> -extern int varobj_update (struct varobj *var, struct varobj ***changelist);
> +extern int varobj_update (struct varobj **varp, struct varobj ***changelist);
> 
>  #endif /* VAROBJ_H */

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: [RFA] varobj.c/h interface change to varobj_update

[Keith Seitz]

On Fri, 17 Aug 2001, Fernando Nasser wrote:

> Keith Seitz wrote:
> >
> > I would like to change the interface for the function varobj_update from
> >
> > int varobj_update (struct varobj *var, struct varobj ***changelist);
> >
> > to
> >
> > int varobj_update (struct varobj **varp, struct varobj ***changelist);
>
> It is OK with me.  I know that you will take care of things if something
> breaks ;-)

Checked in. :-)

Thanks for the quick response!!
Keith

> > ChangeLog
> > 2001-08-17  Keith Seitz  <keiths@redhat.com>
> >
> >         * varobj.c (varobj_update): Change first parameter to
> >         pointer to struct varobj*. This function can delete
> >         varobjs, so we need to give callers the new varobj
> >         when this happens.
> >         (value_of_root): Update "var", too, if "var_handle"
> >         changes.
> >         * varobj.h (varobj_update): Likewise.
> >
> > Patch
> > Index: varobj.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/varobj.c,v
> > retrieving revision 1.19
> > diff -u -p -r1.19 varobj.c
> > --- varobj.c    2001/05/10 16:57:01     1.19
> > +++ varobj.c    2001/08/17 18:13:48
> > @@ -866,10 +866,14 @@ varobj_list (struct varobj ***varlist)
> >      -2 if the type changed
> >      Otherwise it is the number of children + parent changed
> >
> > -   Only root variables can be updated... */
> > +   Only root variables can be updated...
> >
> > +   NOTE: This function may delete the caller's varobj. If it
> > +   returns -2, then it has done this and VARP will be modified
> > +   to point to the new varobj. */
> > +
> >  int
> > -varobj_update (struct varobj *var, struct varobj ***changelist)
> > +varobj_update (struct varobj **varp, struct varobj ***changelist)
> >  {
> >    int changed = 0;
> >    int type_changed;
> > @@ -889,7 +893,7 @@ varobj_update (struct varobj *var, struc
> >      return -1;
> >
> >    /*  Only root variables can be updated... */
> > -  if (var->root->rootvar != var)
> > +  if ((*varp)->root->rootvar != *varp)
> >      /* Not a root var */
> >      return -1;
> >
> > @@ -903,10 +907,10 @@ varobj_update (struct varobj *var, struc
> >       value_of_root variable dispose of the varobj if the type
> >       has changed. */
> >    type_changed = 1;
> > -  new = value_of_root (&var, &type_changed);
> > +  new = value_of_root (varp, &type_changed);
> >    if (new == NULL)
> >      {
> > -      var->error = 1;
> > +      (*varp)->error = 1;
> >        return -1;
> >      }
> >
> > @@ -917,34 +921,34 @@ varobj_update (struct varobj *var, struc
> >       them note that it's changed. */
> >    if (type_changed)
> >      {
> > -      vpush (&result, var);
> > +      vpush (&result, *varp);
> >        changed++;
> >      }
> >    /* If values are not equal, note that it's changed.
> >       There a couple of exceptions here, though.
> >       We don't want some types to be reported as "changed". */
> > -  else if (type_changeable (var) && !my_value_equal (var->value, new, &error2))
> > +  else if (type_changeable (*varp) && !my_value_equal ((*varp)->value, new, &error2))
> >      {
> > -      vpush (&result, var);
> > +      vpush (&result, *varp);
> >        changed++;
> >        /* error2 replaces var->error since this new value
> >           WILL replace the old one. */
> > -      var->error = error2;
> > +      (*varp)->error = error2;
> >      }
> >
> >    /* We must always keep around the new value for this root
> >       variable expression, or we lose the updated children! */
> > -  value_free (var->value);
> > -  var->value = new;
> > +  value_free ((*varp)->value);
> > +  (*varp)->value = new;
> >
> >    /* Initialize a stack */
> >    vpush (&stack, NULL);
> >
> >    /* Push the root's children */
> > -  if (var->children != NULL)
> > +  if ((*varp)->children != NULL)
> >      {
> >        struct varobj_child *c;
> > -      for (c = var->children; c != NULL; c = c->next)
> > +      for (c = (*varp)->children; c != NULL; c = c->next)
> >         vpush (&stack, c->child);
> >      }
> >
> > @@ -1647,6 +1651,7 @@ value_of_root (struct varobj **var_handl
> >             }
> >           install_variable (tmp_var);
> >           *var_handle = tmp_var;
> > +         var = *var_handle;
> >           *type_changed = 1;
> >         }
> >      }
> > Index: varobj.h
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/varobj.h,v
> > retrieving revision 1.3
> > diff -u -p -r1.3 varobj.h
> > --- varobj.h    2001/03/06 08:21:18     1.3
> > +++ varobj.h    2001/08/17 18:13:48
> > @@ -95,6 +95,6 @@ extern int varobj_set_value (struct varo
> >
> >  extern int varobj_list (struct varobj ***rootlist);
> >
> > -extern int varobj_update (struct varobj *var, struct varobj ***changelist);
> > +extern int varobj_update (struct varobj **varp, struct varobj ***changelist);
> >
> >  #endif /* VAROBJ_H */
>
> --
> Fernando Nasser
> Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
> 2323 Yonge Street, Suite #300
> Toronto, Ontario   M4P 2C9
>