Re: [patch 4/8] Types GC [varobj_list to all_root_varobjs]

[Vladimir Prus <vladimir@codesourcery.com>]

On Thursday 02 July 2009 Jan Kratochvil wrote:

> On Tue, 09 Jun 2009 22:50:42 +0200, Tom Tromey wrote:
> > >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> > 
> > Jan> I find the callback based iterator easier to use,
> [...]
> > It seems reasonable to me, though it would be nice to have Volodya's
> > approval.
> 
> Is it OK to check it in, Vladimir?  The patch would go in unchanged:
> 	http://sourceware.org/ml/gdb-patches/2009-05/msg00547.html
> 
> Regression re-tested now on {x86_64,i686}-fedora-linux-gnu.

Is this cleanup-only patch? I am a bit concerned that it appears
to increase code size, and all it does it changes explicit iteration
to callback iteration. Can we just make varobj.c expose vector of
varobjs?

> > Jan> in fact there were bugs due to the
> > Jan> current calling semantics (`floating' lockup, memory leaks).
> > 
> > Details on the bugs that this fixes would be nice.
> > A test case would also be nice, assuming these are testable.
> 
> The `floating' lockup will get fixed by a later patch using this new
> all_root_varobjs function.   A testcase for it was in a now-obsolete patch:
> 	[patch] Fix gdb.mi hang on floating VAROBJs
> 	http://sourceware.org/ml/gdb-patches/2009-05/msg00433.html
> This patch itself still does not fix it.

IIUC, the varobj_invalidate problem can be fixed with a small patch below.
Am I missing something? If no, such a patch is not in any way made easier
by callback iteration.

- Volodya

Index: gdb/varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.137
diff -u -p -r1.137 varobj.c
--- gdb/varobj.c        30 Jun 2009 09:24:47 -0000      1.137
+++ gdb/varobj.c        2 Jul 2009 10:08:22 -0000
@@ -3225,8 +3225,10 @@ varobj_invalidate (void)
          /* Floating varobjs are reparsed on each stop, so we don't care if
             the presently parsed expression refers to something that's gone.
             */
-         if ((*varp)->root->floating)
+         if ((*varp)->root->floating) {
+           varp++;
            continue;
+         }

          /* global var must be re-evaluated.  */
          if ((*varp)->root->valid_block == NULL)