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

[Vladimir Prus <ghost at cs dot msu dot 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