Re: RFC: MI - Detecting change of string contents with variable objects

[Vladimir Prus <ghost@cs.msu.su>]

On Thursday 04 January 2007 09:10, Nick Roberts wrote:


> *** varobj.c    04 Jan 2007 16:22:25 +1300      1.68
> --- varobj.c    04 Jan 2007 19:04:25 +1300      
> *************** struct varobj
> *** 127,132 ****
> --- 127,135 ----
>   
>     /* Was this variable updated via a varobj_set_value operation */
>     int updated;
> + 
> +   /* Last print value */

Dot and two spaces ;-)

> +   char *print_value;
>   };
>   
>   /* Every variable keeps a linked list of its children, described
> *************** static int variable_editable (struct var
> *** 234,239 ****
> --- 237,245 ----
>   
>   static char *my_value_of_variable (struct varobj *var);
>   
> + static char *value_get_print_value (struct value *value,
> +                                   enum varobj_display_formats format);
> + 

I don't see a comment to this function either here or at the definition point.

>   static int type_changeable (struct varobj *var);
>   
>   /* C implementation */
> *************** install_new_value (struct varobj *var, s
> *** 978,1003 ****
>             changed = 1;
>           else
>             {
>               gdb_assert (!value_lazy (var->value));
> -             gdb_assert (!value_lazy (value));

Did you remove this because it failed? If so, I'd like to understand why.
I think this assert is needed -- if the value is lazy, then even if
printing code will fetch the value, you'll be comparing current value with
current value. That's a definite bug, so must be asserted.

>               
> !             if (!value_contents_equal (var->value, value))
> !               changed = 1;
>             }
>         }
>       }
> !     
>     /* We must always keep the new value, since children depend on it.  */
>     if (var->value != NULL)
>       value_free (var->value);
>     var->value = value;
>     var->updated = 0;
> !   
>     gdb_assert (!var->value || value_type (var->value));

Is that a formatting change above?

>   /* Update the values for a variable and its children.  This is a
>      two-pronged attack.  First, re-parse the value for the root's
> --- 984,1017 ----
>             changed = 1;
>           else
>             {
> +             char* print_value = value_get_print_value (value, var->format);
>               gdb_assert (!value_lazy (var->value));
>               
> !             if (var->print_value)
> !               {
> !                 if (strcmp (var->print_value, print_value))

Can you use 

        strcmp (var->print_value, print_value) != 0

for legibility?

> !                   {
> !                     xfree (var->print_value);
> !                     var->print_value = print_value;
> !                     changed = 1;
> !                   }

So, if values differ you xfree the old one and assign the new one. If the
values are the same -- where is 'print_value' freed?

- Volodya