From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30967 invoked by alias); 4 Jan 2007 06:10:40 -0000 Received: (qmail 30952 invoked by uid 22791); 4 Jan 2007 06:10:39 -0000 X-Spam-Check-By: sourceware.org Received: from viper.snap.net.nz (HELO viper.snap.net.nz) (202.37.101.8) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 04 Jan 2007 06:10:33 +0000 Received: from kahikatea.snap.net.nz (p202-124-120-135.snap.net.nz [202.124.120.135]) by viper.snap.net.nz (Postfix) with ESMTP id 004833D834E; Thu, 4 Jan 2007 19:10:28 +1300 (NZDT) Received: by kahikatea.snap.net.nz (Postfix, from userid 500) id 6D0AD4F6CB; Thu, 4 Jan 2007 19:10:27 +1300 (NZDT) From: Nick Roberts MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <17820.39505.966623.305338@kahikatea.snap.net.nz> Date: Thu, 04 Jan 2007 06:10:00 -0000 To: Daniel Jacobowitz Cc: Vladimir Prus , gdb-patches@sources.redhat.com Subject: Re: RFC: MI - Detecting change of string contents with variable objects In-Reply-To: <20070104042038.GA3918@nevyn.them.org> References: <17797.65268.689590.797944@kahikatea.snap.net.nz> <17798.19683.251190.740216@kahikatea.snap.net.nz> <200612181136.02429.ghost@cs.msu.su> <20061218133827.GA24800@nevyn.them.org> <17799.3497.476593.138858@kahikatea.snap.net.nz> <20070103224605.GO17935@nevyn.them.org> <17820.32496.851404.587153@kahikatea.snap.net.nz> <20070104042038.GA3918@nevyn.them.org> X-Mailer: VM 7.19 under Emacs 22.0.92.5 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: 2007-01/txt/msg00104.txt.bz2 > > struct type *type; > > > > + > > /* The value of this expression or subexpression. This may be NULL. > > Please try not to gratuitously move whitespace around (you > did this in a bunch of places). I really appreciate that > when reviewing. Well I try not to do it gratuitously but the above must have been a case of sticky fingers. The others seem sensible and hopefully not too distracting. > > ! if (var->print_value) > > ! { > > ! if (strcmp (var->print_value, > > ! value_get_print_value (value, var->format))) > > ! { > > ! xfree (var->print_value); > > ! var->print_value = > > ! xstrdup (value_get_print_value (value, var->format)); > > ! changed = 1; > > ! } > > ! } > > ! else > > ! var->print_value = > > ! xstrdup (value_get_print_value (value, var->format)); > > value_get_print_value does a bunch of work, and returns a malloced > string. So you should avoid calling it twice, and avoid leaking the > return value. OK, I had thought the return value of value_get_print_value was placed somewhere on the stack. New attempt below. -- Nick http://www.inet.net.nz/~nickrob *** varobj.c 04 Jan 2007 16:22:25 +1300 1.68 --- varobj.c 04 Jan 2007 19:04:25 +1300 *************** struct varobj *** 127,132 **** --- 127,135 ---- /* Was this variable updated via a varobj_set_value operation */ int updated; + + /* Last print value */ + char *print_value; }; /* Every variable keeps a linked list of its children, described *************** static int variable_editable (struct var *** 234,239 **** --- 237,245 ---- static char *my_value_of_variable (struct varobj *var); + static char *value_get_print_value (struct value *value, + enum varobj_display_formats format); + static int type_changeable (struct varobj *var); /* C implementation */ *************** install_new_value (struct varobj *var, s *** 978,1003 **** changed = 1; else { gdb_assert (!value_lazy (var->value)); - gdb_assert (!value_lazy (value)); ! if (!value_contents_equal (var->value, value)) ! changed = 1; } } } ! /* We must always keep the new value, since children depend on it. */ if (var->value != NULL) value_free (var->value); var->value = value; var->updated = 0; ! gdb_assert (!var->value || value_type (var->value)); return changed; } - /* Update the values for a variable and its children. This is a two-pronged attack. First, re-parse the value for the root's --- 984,1017 ---- changed = 1; else { + char* print_value = value_get_print_value (value, var->format); gdb_assert (!value_lazy (var->value)); ! if (var->print_value) ! { ! if (strcmp (var->print_value, print_value)) ! { ! xfree (var->print_value); ! var->print_value = print_value; ! changed = 1; ! } ! } ! else ! var->print_value = print_value; } } } ! /* We must always keep the new value, since children depend on it. */ if (var->value != NULL) value_free (var->value); var->value = value; var->updated = 0; ! gdb_assert (!var->value || value_type (var->value)); return changed; } /* Update the values for a variable and its children. This is a two-pronged attack. First, re-parse the value for the root's *************** new_variable (void) *** 1470,1475 **** --- 1484,1490 ---- var->format = 0; var->root = NULL; var->updated = 0; + var->print_value = NULL; return var; } *************** free_variable (struct varobj *var) *** 1503,1508 **** --- 1518,1524 ---- xfree (var->name); xfree (var->obj_name); + xfree (var->print_value); xfree (var); } *************** my_value_of_variable (struct varobj *var *** 1785,1790 **** --- 1801,1820 ---- return (*var->root->lang->value_of_variable) (var); } + static char * + value_get_print_value (struct value *value, enum varobj_display_formats format) + { + long dummy; + struct ui_file *stb = mem_fileopen (); + struct cleanup *old_chain = make_cleanup_ui_file_delete (stb); + char *thevalue; + + common_val_print (value, stb, format_code[(int) format], 1, 0, 0); + thevalue = ui_file_xstrdup (stb, &dummy); + do_cleanups (old_chain); + return thevalue; + } + /* Return non-zero if changes in value of VAR must be detected and reported by -var-update. Return zero is -var-update should never report *************** c_value_of_variable (struct varobj *var) *** 2153,2171 **** } else { - long dummy; - struct ui_file *stb = mem_fileopen (); - struct cleanup *old_chain = make_cleanup_ui_file_delete (stb); - char *thevalue; - gdb_assert (type_changeable (var)); gdb_assert (!value_lazy (var->value)); ! common_val_print (var->value, stb, ! format_code[(int) var->format], 1, 0, 0); ! thevalue = ui_file_xstrdup (stb, &dummy); ! do_cleanups (old_chain); ! return thevalue; ! } } } } --- 2183,2192 ---- } else { gdb_assert (type_changeable (var)); gdb_assert (!value_lazy (var->value)); ! return value_get_print_value (var->value, var->format); ! } } } }