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

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

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


> > 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 and approve the
fixup + a testcase below.

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


Thanks,
Jan


gdb/
2009-07-02  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* mi/mi-cmd-var.c (mi_cmd_var_update): Replace a while loop by for loop.
	* varobj.c (varobj_invalidate): Replace a while loop by for loop.

gdb/testsuite/
2009-05-25  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.mi/mi2-var-cmd.exp (floating varobj invalidation): New test.

--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -606,8 +606,7 @@ mi_cmd_var_update (char *command, char **argv, int argc)
 	  do_cleanups (cleanup);
 	  return;
 	}
-      cr = rootlist;
-      while (*cr != NULL)
+      for (cr = rootlist; *cr != NULL; cr++)
 	{
 	  int thread_id = varobj_get_thread_id (*cr);
 	  int thread_stopped = 0;
@@ -624,7 +623,6 @@ mi_cmd_var_update (char *command, char **argv, int argc)
 	  if (thread_stopped)
 	    if (*name == '*' || varobj_floating_p (*cr))
 	      varobj_update_one (*cr, print_values, 0 /* implicit */);
-	  cr++;
 	}
       do_cleanups (cleanup);
     }
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -3226,16 +3226,13 @@ varobj_invalidate (void)
 
   if (varobj_list (&all_rootvarobj) > 0)
     {
-      varp = all_rootvarobj;
-      while (*varp != NULL)
+      for (varp = all_rootvarobj; *varp != NULL; varp++)
 	{
 	  /* 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) {
-	    varp++;
+	  if ((*varp)->root->floating)
 	    continue;
-	  }
 
 	  /* global var must be re-evaluated.  */     
 	  if ((*varp)->root->valid_block == NULL)
@@ -3257,8 +3254,6 @@ varobj_invalidate (void)
 	    }
 	  else /* locals must be invalidated.  */
 	    (*varp)->root->is_valid = 0;
-
-	  varp++;
 	}
     }
   xfree (all_rootvarobj);

--- a/gdb/testsuite/gdb.mi/mi2-var-cmd.exp
+++ b/gdb/testsuite/gdb.mi/mi2-var-cmd.exp
@@ -523,5 +523,9 @@ mi_gdb_test "-var-update selected_a" \
 	"\\^done,changelist=\\\[\{name=\"selected_a\",in_scope=\"true\",type_changed=\"true\",new_type=\"int\",new_num_children=\"0\"\}\\\]" \
 	"update selected_a in do_special_tests"
 
+mi_gdb_test "-file-exec-and-symbols ${binfile}" "\\^done" \
+	    "floating varobj invalidation"
+
+
 mi_gdb_exit
 return 0