I have fixed the patch and checked it in.  Final version is attached.

On Wednesday 12 June 2002 07:24 pm, Andrew Cagney wrote:
> > Anything using varobj is randomly corrupting memory and will cause
> > crashes in Insight or anything using mi varobjs.  This patch fixes that
> > and some other minor problems.
>
> Hmm, I'm not seeing MI test failures.  Would you, by chance, have
> something reproduceable for an MI testcase?

Memory corruption bugs are not easy to reproduce.  I never even had a 
reproduceable test case for Insight.  The best I could do was a sequence of a 
dozen or so operations that crashed most of the time. I just used valgrind 
and tracked down all the offending code. 

> Anyway, separating out the changes:
>
> I consider the sprintf() -> xasprintf() transformations:
> > (varobj_gen_name): Use xasprintf.
> > 	(create_child): Use xasprintf.
>
> to be ``obvious'' and can, separatly, go straight in (only ~300 other
> calls to go ...).

OK.

> The frame_id stuff:
> >   /* Save the selected stack frame, since we will need to change it
> >       in order to evaluate expressions. */
> > -  old_fi = selected_frame;
> > +  get_frame_id (selected_frame, &old_fid);
>
> is fine except I'm not sure about:
> > -  var->root->frame = (CORE_ADDR) -1;
> > +  var->root->frame.base = (CORE_ADDR) -1;
> > +  var->root->frame.pc = (CORE_ADDR) -1;
>
> The function:
>
> frame_find_by_id (struct frame_id id)
>
> has:
>
>    /* ZERO denotes the null frame, let the caller decide what to do
>       about it.  Should it instead return get_current_frame()?  */
>    if (id.base == 0 && id.pc == 0)
>      return NULL;
>
> (see find_frame_addr_in_frame_chain for where this came from) so I think
> zero would be better.

Agreed. Changed in the patch.

>
> For the indentation changes, 

They were accidentally included and are removed from the patch.

-- 
Martin Hunt
GDB Engineer
Red Hat, Inc.