From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13412 invoked by alias); 4 Jul 2009 21:11:48 -0000 Received: (qmail 13399 invoked by uid 22791); 4 Jul 2009 21:11:47 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_32,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 04 Jul 2009 21:11:40 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n64L9cgN016115; Sat, 4 Jul 2009 17:09:38 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n64L9bQs014927; Sat, 4 Jul 2009 17:09:37 -0400 Received: from host0.dyn.jankratochvil.net (sebastian-int.corp.redhat.com [172.16.52.221]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n64L9VCo003891; Sat, 4 Jul 2009 17:09:35 -0400 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.3/8.14.3) with ESMTP id n64L9TZo005984; Sat, 4 Jul 2009 23:09:29 +0200 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.3/8.14.3/Submit) id n64L9CPc005959; Sat, 4 Jul 2009 23:09:12 +0200 Date: Sat, 04 Jul 2009 21:11:00 -0000 From: Jan Kratochvil To: Vladimir Prus Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [patch 4/8] Types GC [varobj_list to all_root_varobjs] Message-ID: <20090704210912.GA1600@host0.dyn.jankratochvil.net> References: <20090525080233.GD13323@host0.dyn.jankratochvil.net> <20090702083705.GA14783@host0.dyn.jankratochvil.net> <200907021409.39886.vladimir@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200907021409.39886.vladimir@codesourcery.com> User-Agent: Mutt/1.5.19 (2009-01-05) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-07/txt/msg00106.txt.bz2 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 * 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 * 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