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

[Vladimir Prus <vladimir@codesourcery.com>]

[-- Attachment #1: Type: Text/Plain, Size: 1626 bytes --]

On Sunday 05 July 2009 Jan Kratochvil wrote:

> On Thu, 02 Jul 2009 12:09:39 +0200, Vladimir Prus wrote:
> > On Thursday 02 July 2009 Jan Kratochvil wrote:
> > > Is it OK to check it in, Vladimir?  The patch would go in unchanged:
> > > 	http://sourceware.org/ml/gdb-patches/2009-05/msg00547.html
> > 
> > Is this cleanup-only patch?
> 
> Yes.
> 
> > I am a bit concerned that it appears to increase code size,
> 
> all_root_varobjs fully replaced the varobj_list function which just could not
> be deleted as it was still used by varobj_invalidate.  varobj_invalidate was
> rewritten in a later patch where varobj_list could be finally dropped:
> 	[patch 8/8] Types GC [varobj]
> 	http://sourceware.org/ml/gdb-patches/2009-05/msg00551.html

Is that patch awaiting review, or some newer version is forthcoming?

> > > 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.
> 
> Yes, such patch works.
> 
> Sending also a code style fixup on top of your fix to make the code more safe
> preventing such errors in the future.
> 
> Please provide a ChangeLog entry to your fix, check it in 

It now checked in, as attached.

> and approve the
> fixup + a testcase below.

This is OK, thanks.

> The all_root_varobjs patch / VEC rewrite I will re-send afterwards.

OK.

Thanks,
Volodya

[-- Attachment #2: hang.diff --]
[-- Type: text/x-patch, Size: 1288 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.10694
diff -u -p -r1.10694 ChangeLog
--- ChangeLog	7 Jul 2009 08:47:09 -0000	1.10694
+++ ChangeLog	7 Jul 2009 08:50:02 -0000
@@ -1,5 +1,12 @@
 2009-07-07  Vladimir Prus  <vladimir@codesourcery.com>
 
+	Fix hang in floating varobjs.
+
+	* varob.c (varobj_invalidate): Advance to next
+	element when processing floating varobj.
+
+2009-07-07  Vladimir Prus  <vladimir@codesourcery.com>
+
 	* varobj.c: Remove unnecessary include.
 
 2009-07-07  Tristan Gingold  <gingold@adacore.com>
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.140
diff -u -p -r1.140 varobj.c
--- varobj.c	7 Jul 2009 08:47:10 -0000	1.140
+++ varobj.c	7 Jul 2009 08:50:02 -0000
@@ -3231,8 +3231,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)